6:08 p.m.
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
6:08 p.m.
New subject: [PATCH 1/3] qemu: eliminate use of virDomainNetDefClear() in qemuConnectDomainXMLToNative()
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(a)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
6:08 p.m.
New subject: [PATCH 2/3] conf: eliminate virDomainNetDefClear()
This function is no longer used anywhere except virDomainNetDefFree(),
so just inline its contents there.
Signed-off-by: Laine Stump <laine(a)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
6:08 p.m.
New subject: [PATCH 3/3] conf: use g_free() instead of VIR_FREE in virDomainNetDefFree()
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(a)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
2:09 a.m.
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(a)redhat.com>
Michal
1512
days inactive
1513
days old
4 comments
2 participants
participants (2)
-
Laine Stump -
Michal Prívozník