[PATCH 0/3] get rid of virDomainNetDefClear()
While looking for something else, I noticed that there were only 2 callers to virDomainNetDefClear(), and one of those was virDomain NetDefFree(). virDomainNetDefClear() necessarily has a lot of VIR_FREE() in it that couldn't be changed to g_free() (because it's possible the object will be used after it is cleared). This series eliminates the actual practical usage of virDomainNetDefClear() by just creating a new object instead of clearing and re-using the old object, then moves the contents of virDomainNetDefClear() into virDomainNetDefFree(), and finally changes all the uses of VIR_FREE() to g_free() (now that it's safe to do so). Laine Stump (3): qemu: eliminate use of virDomainNetDefClear() in qemuConnectDomainXMLToNative() conf: eliminate virDomainNetDefClear() conf: use g_free() instead of VIR_FREE in virDomainNetDefFree() src/conf/domain_conf.c | 51 +++++++++++++++++----------------------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 26 +++++++++----------- 4 files changed, 33 insertions(+), 46 deletions(-) -- 2.26.2
Instead of saving the interesting pieces of each existing NetDef, clearing it, and then copying back the saved pieces after setting the type to ethernet, just create a new NetDef, copy in the interesting bits, and replace the old one. (The end game is to eliminate virDomainNetDefClear() completely, since this is the only real use) Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_driver.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b27f05992b..19e2aff3e4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6456,24 +6456,20 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, */ for (i = 0; i < vm->def->nnets; i++) { virDomainNetDefPtr net = vm->def->nets[i]; - unsigned int bootIndex = net->info.bootIndex; - g_autofree char *model = NULL; - virMacAddr mac = net->mac; - char *script = net->script; + virDomainNetDefPtr newNet = virDomainNetDefNew(driver->xmlopt); - model = g_strdup(virDomainNetGetModelString(net)); - - net->script = NULL; - - virDomainNetDefClear(net); + if (!newNet) + goto cleanup; - net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; - net->info.bootIndex = bootIndex; - net->mac = mac; - net->script = script; + newNet->type = VIR_DOMAIN_NET_TYPE_ETHERNET; + newNet->info.bootIndex = net->info.bootIndex; + newNet->model = net->model; + newNet->modelstr = g_steal_pointer(&net->modelstr); + newNet->mac = net->mac; + newNet->script = g_steal_pointer(&net->script); - if (virDomainNetSetModelString(net, model) < 0) - goto cleanup; + virDomainNetDefFree(net); + vm->def->nets[i] = newNet; } if (!(cmd = qemuProcessCreatePretendCmd(driver, vm, NULL, -- 2.26.2
This function is no longer used anywhere except virDomainNetDefFree(), so just inline its contents there. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 9 +-------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a91dbd4aa9..df5171301d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2491,7 +2491,7 @@ virDomainVsockDefFree(virDomainVsockDefPtr vsock) void -virDomainNetDefClear(virDomainNetDefPtr def) +virDomainNetDefFree(virDomainNetDefPtr def) { if (!def) return; @@ -2566,14 +2566,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) virNetDevBandwidthFree(def->bandwidth); def->bandwidth = NULL; virNetDevVlanClear(&def->vlan); -} -void -virDomainNetDefFree(virDomainNetDefPtr def) -{ - if (!def) - return; - virDomainNetDefClear(def); virObjectUnref(def->privateData); VIR_FREE(def); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8f1662aae0..9a44315519 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3056,7 +3056,6 @@ void virDomainActualNetDefFree(virDomainActualNetDefPtr def); virDomainVsockDefPtr virDomainVsockDefNew(virDomainXMLOptionPtr xmlopt); void virDomainVsockDefFree(virDomainVsockDefPtr vsock); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVsockDef, virDomainVsockDefFree); -void virDomainNetDefClear(virDomainNetDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6bcbfa667d..4e66385bab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -507,7 +507,6 @@ virDomainNetARPInterfaces; virDomainNetBandwidthUpdate; virDomainNetDefActualFromNetworkPort; virDomainNetDefActualToNetworkPort; -virDomainNetDefClear; virDomainNetDefFormat; virDomainNetDefFree; virDomainNetDefNew; -- 2.26.2
All these lines were moved over from the now-defunct virDomainNetDefClear(), which required all pointers to be cleared after free, but virDomainNetDefFree() doesn't have that restriction - after free'ing the pointers are never again referenced, so g_free() is safe. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index df5171301d..7292dceb6b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2509,27 +2509,27 @@ virDomainNetDefFree(virDomainNetDefPtr def) case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_UDP: - VIR_FREE(def->data.socket.address); - VIR_FREE(def->data.socket.localaddr); + g_free(def->data.socket.address); + g_free(def->data.socket.localaddr); break; case VIR_DOMAIN_NET_TYPE_NETWORK: - VIR_FREE(def->data.network.name); - VIR_FREE(def->data.network.portgroup); + g_free(def->data.network.name); + g_free(def->data.network.portgroup); virDomainActualNetDefFree(def->data.network.actual); def->data.network.actual = NULL; break; case VIR_DOMAIN_NET_TYPE_BRIDGE: - VIR_FREE(def->data.bridge.brname); + g_free(def->data.bridge.brname); break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - VIR_FREE(def->data.internal.name); + g_free(def->data.internal.name); break; case VIR_DOMAIN_NET_TYPE_DIRECT: - VIR_FREE(def->data.direct.linkdev); + g_free(def->data.direct.linkdev); break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: @@ -2542,24 +2542,24 @@ virDomainNetDefFree(virDomainNetDefPtr def) break; } - VIR_FREE(def->backend.tap); - VIR_FREE(def->backend.vhost); - VIR_FREE(def->teaming.persistent); - VIR_FREE(def->virtPortProfile); - VIR_FREE(def->script); - VIR_FREE(def->downscript); - VIR_FREE(def->domain_name); - VIR_FREE(def->ifname); - VIR_FREE(def->ifname_guest); - VIR_FREE(def->ifname_guest_actual); - VIR_FREE(def->virtio); - VIR_FREE(def->coalesce); + g_free(def->backend.tap); + g_free(def->backend.vhost); + g_free(def->teaming.persistent); + g_free(def->virtPortProfile); + g_free(def->script); + g_free(def->downscript); + g_free(def->domain_name); + g_free(def->ifname); + g_free(def->ifname_guest); + g_free(def->ifname_guest_actual); + g_free(def->virtio); + g_free(def->coalesce); virNetDevIPInfoClear(&def->guestIP); virNetDevIPInfoClear(&def->hostIP); virDomainDeviceInfoClear(&def->info); - VIR_FREE(def->filter); + g_free(def->filter); virHashFree(def->filterparams); def->filterparams = NULL; @@ -2568,7 +2568,7 @@ virDomainNetDefFree(virDomainNetDefPtr def) virNetDevVlanClear(&def->vlan); virObjectUnref(def->privateData); - VIR_FREE(def); + g_free(def); } -- 2.26.2
On 10/1/20 1:08 AM, Laine Stump wrote:
While looking for something else, I noticed that there were only 2 callers to virDomainNetDefClear(), and one of those was virDomain NetDefFree(). virDomainNetDefClear() necessarily has a lot of VIR_FREE() in it that couldn't be changed to g_free() (because it's possible the object will be used after it is cleared). This series eliminates the actual practical usage of virDomainNetDefClear() by just creating a new object instead of clearing and re-using the old object, then moves the contents of virDomainNetDefClear() into virDomainNetDefFree(), and finally changes all the uses of VIR_FREE() to g_free() (now that it's safe to do so).
Laine Stump (3): qemu: eliminate use of virDomainNetDefClear() in qemuConnectDomainXMLToNative() conf: eliminate virDomainNetDefClear() conf: use g_free() instead of VIR_FREE in virDomainNetDefFree()
src/conf/domain_conf.c | 51 +++++++++++++++++----------------------- src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 26 +++++++++----------- 4 files changed, 33 insertions(+), 46 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Laine Stump -
Michal Prívozník