[PATCH 0/2] fix re-attach of bridge device for <interface type='bridge'>
For a long time I thought this worked, but during discussion in a bugzilla report one day I realized it only worked properly for <interface type='network'> - we're not even trying to reattach bridges during libvirtd restart for type='bridge' (i.e. no associated libvirt network). These two patches together fix that problem. Laine Stump (2): call virDomainNetNotifyActualDevice() for all interface types use g_autoptr for (almost) all virConnectPtrs used with virGetConnectNetwork() src/conf/domain_conf.c | 3 +-- src/libxl/libxl_driver.c | 24 +++++++++--------------- src/lxc/lxc_driver.c | 17 +++++------------ src/lxc/lxc_process.c | 19 +++++++------------ src/qemu/qemu_process.c | 9 ++++----- 5 files changed, 26 insertions(+), 46 deletions(-) -- 2.29.2
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@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
two occurences (in libxl_domain.c) couldn't be changed due to a compile error. I'll send those in a separate mail looking for an explanation... Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 3 +-- src/libxl/libxl_driver.c | 15 +++++---------- src/lxc/lxc_driver.c | 17 +++++------------ src/lxc/lxc_process.c | 10 +++------- 4 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d90945b1d8..bdd695322b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30966,7 +30966,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())) @@ -30986,7 +30986,6 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, ret = 0; cleanup: - virObjectUnref(conn); virTypedParamsFree(params, nparams); virObjectUnref(port); virObjectUnref(net); 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
On a Friday in 2021, Laine Stump wrote:
two occurences (in libxl_domain.c) couldn't be changed due to a
*occurrences
compile error. I'll send those in a separate mail looking for an explanation...
or just #include "datatypes.h" and make the changes in the same patch.
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 3 +-- src/libxl/libxl_driver.c | 15 +++++---------- src/lxc/lxc_driver.c | 17 +++++------------ src/lxc/lxc_process.c | 10 +++------- 4 files changed, 14 insertions(+), 31 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On 1/8/21 8:42 AM, Laine Stump wrote:
For a long time I thought this worked, but during discussion in a bugzilla report one day I realized it only worked properly for <interface type='network'> - we're not even trying to reattach bridges during libvirtd restart for type='bridge' (i.e. no associated libvirt network).
These two patches together fix that problem.
Laine Stump (2): call virDomainNetNotifyActualDevice() for all interface types use g_autoptr for (almost) all virConnectPtrs used with virGetConnectNetwork()
src/conf/domain_conf.c | 3 +-- src/libxl/libxl_driver.c | 24 +++++++++--------------- src/lxc/lxc_driver.c | 17 +++++------------ src/lxc/lxc_process.c | 19 +++++++------------ src/qemu/qemu_process.c | 9 ++++----- 5 files changed, 26 insertions(+), 46 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Ján Tomko -
Laine Stump -
Michal Privoznik