9:38 a.m.
It was late (early) when I sent this the first time, and the final
patch (which was just a cleanup to use g_autoptr in a few places not
even needed for the declared reason for the patches) wasn't compiling
correctly. I intended to send the 1st and 2nd patches on the branch
and forget about the 3rd (g_autoptr) until later, but instead
accidentally sent patches 2 & 3.
jtomko pointed out wh patch 3 hadn't worked properly, so I've updated
that, added a simple patch to eliminate a superfluous cleanup: label,
and am re-sending with with what *should have been* patch 1 included
for the first time.
If that's all too complicated:
* Patch 1 was accidentally not sent in V1
* Patch 2 was acked (twice even! :-)) and hasn't changed
* Patch 3 just adds g_autoptr(virConnect) in libxl_domain.c (now that
jtomko pointed out the #include of datatypes.h was missing), but
otherwise unchanged
* Patch 4 is trivial but new.
Laine Stump (4):
conf: make virDomainNetNotifyActualDevice() callable for all interface
types
call virDomainNetNotifyActualDevice() for all interface types
use g_autoptr for all virConnectPtrs used with virGetConnectNetwork()
libxl: remove a now-unnecessary ret variable and cleanup: label.
src/conf/domain_conf.c | 10 +++++-----
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_domain.c | 18 +++++++-----------
src/libxl/libxl_driver.c | 24 +++++++++---------------
src/lxc/lxc_driver.c | 17 +++++------------
src/lxc/lxc_process.c | 19 +++++++------------
src/qemu/qemu_process.c | 9 ++++-----
7 files changed, 38 insertions(+), 61 deletions(-)
--
2.29.2
9:38 a.m.
New subject: [libvirt Patch v2 1/4] conf: make virDomainNetNotifyActualDevice() callable for all interface types
The bridge reattach functionality in this function should be called
for interface types other than just type='network', so make it
callable for any type - it just becomes a NOP for types where no
action is needed.
In the case of <interface type='network'> we need to create a port in
the network driver, and for both type='network and type='bridge' we
need to reattach the bridge device (note that
virDomainNetGetActualBridgeName() gets the bridge name from the
appropriate (and different!) location for either type of interface).
All other interfaces currently require no action.
modifying callers of this function to actually call it for all
interface types is in the next patch. For now the behavior should be
identical pre and post-patch.
(NB: the conn argument can now legitimately be NULL, so we need to
change the ATTRIBUTE_NONNULL() directive for the function's
declaration - I noticed when making this change that argument 3 (the
NetDefPtr) could never be NULL, so I added ATTRIBUTE_NONNULL(3) while
removing ATTRIBUTE_NONNULL(1) (conn)).
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_conf.c | 7 ++++---
src/conf/domain_conf.h | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 472ce79e67..40ca20be33 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30933,8 +30933,9 @@ virDomainNetNotifyActualDevice(virConnectPtr conn,
{
virDomainNetType actualType = virDomainNetGetActualType(iface);
- if (virDomainNetCreatePort(conn, dom, iface,
- VIR_NETWORK_PORT_CREATE_RECLAIM) < 0) {
+ if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && conn
+ && virDomainNetCreatePort(conn, dom, iface,
+ VIR_NETWORK_PORT_CREATE_RECLAIM) < 0) {
return;
}
@@ -30946,7 +30947,7 @@ virDomainNetNotifyActualDevice(virConnectPtr conn,
* (final arg to virNetDevTapReattachBridge())
*/
ignore_value(virNetDevTapReattachBridge(iface->ifname,
-
iface->data.network.actual->data.bridge.brname,
+ virDomainNetGetActualBridgeName(iface),
&iface->mac, dom->uuid,
virDomainNetGetActualVirtPortProfile(iface),
virDomainNetGetActualVlan(iface),
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 59bf8a12b3..ec43bbe186 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3875,7 +3875,7 @@ void
virDomainNetNotifyActualDevice(virConnectPtr conn,
virDomainDefPtr dom,
virDomainNetDefPtr iface)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int
virDomainNetReleaseActualDevice(virConnectPtr conn,
--
2.29.2
9:38 a.m.
New subject: [libvirt Patch v2 2/4] call virDomainNetNotifyActualDevice() for all interface types
Now that this function can be called regardless of interface type (and
whether or not we have a conn for the network driver), let's actually
call it for all interface types. This will assure that we re-connect
any disconnected bridge devices for <interface type='bridge'> as
mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1730084#c26
(until now we've only been reconnecting bridge devices for <interface
type='network'>)
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/libxl/libxl_driver.c | 9 ++++-----
src/lxc/lxc_process.c | 9 ++++-----
src/qemu/qemu_process.c | 9 ++++-----
3 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d58af1a869..40d8c3d174 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -367,11 +367,10 @@ libxlReconnectNotifyNets(virDomainDefPtr def)
if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
virNetDevReserveName(net->ifname);
- if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (!conn && !(conn = virGetConnectNetwork()))
- continue;
- virDomainNetNotifyActualDevice(conn, def, net);
- }
+ if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && !conn)
+ conn = virGetConnectNetwork();
+
+ virDomainNetNotifyActualDevice(conn, def, net);
}
virObjectUnref(conn);
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 2266e2cc40..0b6895bbd4 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1663,11 +1663,10 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
break;
}
- if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (!conn && !(conn = virGetConnectNetwork()))
- continue;
- virDomainNetNotifyActualDevice(conn, def, net);
- }
+ if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && !conn)
+ conn = virGetConnectNetwork();
+
+ virDomainNetNotifyActualDevice(conn, def, net);
}
virObjectUnref(conn);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f0a698ba3d..202d867289 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3411,11 +3411,10 @@ qemuProcessNotifyNets(virDomainDefPtr def)
break;
}
- if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (!conn && !(conn = virGetConnectNetwork()))
- continue;
- virDomainNetNotifyActualDevice(conn, def, net);
- }
+ if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && !conn)
+ conn = virGetConnectNetwork();
+
+ virDomainNetNotifyActualDevice(conn, def, net);
}
}
--
2.29.2
9:38 a.m.
New subject: [libvirt Patch v2 3/4] use g_autoptr for all virConnectPtrs used with virGetConnectNetwork()
Signed-off-by: Laine Stump <laine(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_conf.c | 3 +--
src/libxl/libxl_domain.c | 7 +++----
src/libxl/libxl_driver.c | 15 +++++----------
src/lxc/lxc_driver.c | 17 +++++------------
src/lxc/lxc_process.c | 10 +++-------
5 files changed, 17 insertions(+), 35 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40ca20be33..349fc28c2a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31040,7 +31040,7 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
virNetworkPortPtr port = NULL;
virTypedParameterPtr params = NULL;
int nparams = 0;
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
int ret = -1;
if (!(conn = virGetConnectNetwork()))
@@ -31060,7 +31060,6 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
ret = 0;
cleanup:
- virObjectUnref(conn);
virTypedParamsFree(params, nparams);
virObjectUnref(port);
virObjectUnref(net);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ab838b317c..c046fcb3f7 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -25,6 +25,7 @@
#include "libxl_domain.h"
#include "libxl_capabilities.h"
+#include "datatypes.h"
#include "viralloc.h"
#include "virfile.h"
#include "virerror.h"
@@ -849,7 +850,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
char *file;
virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
#ifdef LIBXL_HAVE_PVUSB
hostdev_flags |= VIR_HOSTDEV_SP_USB;
@@ -936,7 +937,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
}
virDomainObjRemoveTransientDef(vm);
- virObjectUnref(conn);
}
/*
@@ -1050,7 +1050,7 @@ static int
libxlNetworkPrepareDevices(virDomainDefPtr def)
{
size_t i;
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
int ret = -1;
for (i = 0; i < def->nnets; i++) {
@@ -1096,7 +1096,6 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
ret = 0;
cleanup:
- virObjectUnref(conn);
return ret;
}
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 40d8c3d174..5bd3614e21 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -355,7 +355,7 @@ static void
libxlReconnectNotifyNets(virDomainDefPtr def)
{
size_t i;
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
@@ -372,8 +372,6 @@ libxlReconnectNotifyNets(virDomainDefPtr def)
virDomainNetNotifyActualDevice(conn, def, net);
}
-
- virObjectUnref(conn);
}
@@ -3403,7 +3401,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
libxl_device_nic nic;
int ret = -1;
char mac[VIR_MAC_STRING_BUFLEN];
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
virErrorPtr save_err = NULL;
libxl_device_nic_init(&nic);
@@ -3478,7 +3476,6 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK && conn)
virDomainNetReleaseActualDevice(conn, vm->def, net);
}
- virObjectUnref(conn);
virObjectUnref(cfg);
virErrorRestore(&save_err);
return ret;
@@ -3904,13 +3901,11 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
libxl_device_nic_dispose(&nic);
if (!ret) {
if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- virConnectPtr conn = virGetConnectNetwork();
- if (conn) {
+ g_autoptr(virConnect) conn = virGetConnectNetwork();
+ if (conn)
virDomainNetReleaseActualDevice(conn, vm->def, detach);
- virObjectUnref(conn);
- } else {
+ else
VIR_WARN("Unable to release network device '%s'",
NULLSTR(detach->ifname));
- }
}
virDomainNetRemove(vm->def, detachidx);
}
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 88d3890de7..4416acf923 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3463,14 +3463,9 @@ lxcDomainAttachDeviceNetLive(virLXCDriverPtr driver,
* to the one defined in the network definition.
*/
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- virConnectPtr netconn = virGetConnectNetwork();
- if (!netconn)
+ g_autoptr(virConnect) netconn = virGetConnectNetwork();
+ if (!netconn || virDomainNetAllocateActualDevice(netconn, vm->def, net) <
0)
return -1;
- if (virDomainNetAllocateActualDevice(netconn, vm->def, net) < 0) {
- virObjectUnref(netconn);
- return -1;
- }
- virObjectUnref(netconn);
}
/* final validation now that actual type is known */
@@ -4028,13 +4023,11 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
if (!ret) {
virErrorPreserveLast(&save_err);
if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- virConnectPtr conn = virGetConnectNetwork();
- if (conn) {
+ g_autoptr(virConnect) conn = virGetConnectNetwork();
+ if (conn)
virDomainNetReleaseActualDevice(conn, vm->def, detach);
- virObjectUnref(conn);
- } else {
+ else
VIR_WARN("Unable to release network device '%s'",
NULLSTR(detach->ifname));
- }
}
virDomainNetRemove(vm->def, detachidx);
virDomainNetDefFree(detach);
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 0b6895bbd4..cbc04a3dcd 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -172,7 +172,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
virLXCDomainObjPrivatePtr priv = vm->privateData;
const virNetDevVPortProfile *vport = NULL;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
VIR_DEBUG("Cleanup VM name=%s pid=%d reason=%d flags=0x%x",
vm->def->name, (int)vm->pid, (int)reason, flags);
@@ -281,7 +281,6 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
virDomainObjRemoveTransientDef(vm);
virObjectUnref(cfg);
- virObjectUnref(conn);
}
@@ -571,7 +570,7 @@ virLXCProcessSetupInterfaces(virLXCDriverPtr driver,
size_t niface = 0;
virDomainNetDefPtr net;
virDomainNetType type;
- virConnectPtr netconn = NULL;
+ g_autoptr(virConnect) netconn = NULL;
virErrorPtr save_err = NULL;
*veths = g_new0(char *, def->nnets + 1);
@@ -680,7 +679,6 @@ virLXCProcessSetupInterfaces(virLXCDriverPtr driver,
}
virErrorRestore(&save_err);
}
- virObjectUnref(netconn);
return ret;
}
@@ -1634,7 +1632,7 @@ static void
virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
{
size_t i;
- virConnectPtr conn = NULL;
+ g_autoptr(virConnect) conn = NULL;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
@@ -1668,8 +1666,6 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
virDomainNetNotifyActualDevice(conn, def, net);
}
-
- virObjectUnref(conn);
}
--
2.29.2
9:38 a.m.
New subject: [libvirt Patch v2 4/4] libxl: remove a now-unnecessary ret variable and cleanup: label.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/libxl/libxl_domain.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index c046fcb3f7..afa21bf02e 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1051,7 +1051,6 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
{
size_t i;
g_autoptr(virConnect) conn = NULL;
- int ret = -1;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
@@ -1063,9 +1062,9 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
*/
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (!conn && !(conn = virGetConnectNetwork()))
- goto cleanup;
+ return -1;
if (virDomainNetAllocateActualDevice(conn, def, net) < 0)
- goto cleanup;
+ return -1;
}
/* final validation now that actual type is known */
@@ -1090,13 +1089,11 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN;
if (virDomainHostdevInsert(def, hostdev) < 0)
- goto cleanup;
+ return -1;
}
}
- ret = 0;
- cleanup:
- return ret;
+ return 0;
}
static void
--
2.29.2
10:01 a.m.
On 1/8/21 4:38 PM, Laine Stump wrote:
It was late (early) when I sent this the first time, and the final
patch (which was just a cleanup to use g_autoptr in a few places not
even needed for the declared reason for the patches) wasn't compiling
correctly. I intended to send the 1st and 2nd patches on the branch
and forget about the 3rd (g_autoptr) until later, but instead
accidentally sent patches 2 & 3.
jtomko pointed out wh patch 3 hadn't worked properly, so I've updated
that, added a simple patch to eliminate a superfluous cleanup: label,
and am re-sending with with what *should have been* patch 1 included
for the first time.
If that's all too complicated:
* Patch 1 was accidentally not sent in V1
* Patch 2 was acked (twice even! :-)) and hasn't changed
* Patch 3 just adds g_autoptr(virConnect) in libxl_domain.c (now that
jtomko pointed out the #include of datatypes.h was missing), but
otherwise unchanged
* Patch 4 is trivial but new.
Laine Stump (4):
conf: make virDomainNetNotifyActualDevice() callable for all interface
types
call virDomainNetNotifyActualDevice() for all interface types
use g_autoptr for all virConnectPtrs used with virGetConnectNetwork()
libxl: remove a now-unnecessary ret variable and cleanup: label.
src/conf/domain_conf.c | 10 +++++-----
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_domain.c | 18 +++++++-----------
src/libxl/libxl_driver.c | 24 +++++++++---------------
src/lxc/lxc_driver.c | 17 +++++------------
src/lxc/lxc_process.c | 19 +++++++------------
src/qemu/qemu_process.c | 9 ++++-----
7 files changed, 38 insertions(+), 61 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1417
days inactive
1417
days old
5 comments
2 participants
participants (2)
-
Laine Stump -
Michal Privoznik