Re: [PATCH RESEND 16/20] virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFindIndex()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
We're going to need a way to remove a PCI Device from a list without having
a valid virPCIDevicePtr, because the device is missing from the host. This
means that virPCIDevicesListDel() must operate with a PCI Device address
instead.
Turns out that virPCIDevicesListDel() and its related functions only use
the virPCIDeviceAddressPtr of the virPCIDevicePtr, so this change is
simple to do and will not cause hassle in all other callers. Let's
start adapting virPCIDeviceListFindIndex() and crawl our way up to
virPCIDevicesListDel().
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Laine Stump
(also Declared-should-have-been-done-this-way-in-the-first-place-by:
Laine Stump :-))
---
src/util/virpci.c | 15 ---
src/util/virpci.h | 2 +-
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 7143380348..1554acffb6 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1745,7 +1745,7 @@ virPCIDevicePtr
virPCIDeviceListSteal(virPCIDeviceListPtr list,
virPCIDevicePtr dev)
{
-return virPCIDeviceListStealIndex(list, virPCIDeviceListFindIndex(list,
dev));
+return virPCIDeviceListStealIndex(list, virPCIDeviceListFindIndex(list,
>address));
}
void
@@ -1756,16 +1756,17 @@ virPCIDeviceListDel(virPCIDeviceListPtr list,
}
int
-virPCIDeviceListFindIndex(virPCIDeviceListPtr list, virPCIDevicePtr dev)
+virPCIDeviceListFindIndex(virPCIDeviceListPtr list,
+ virPCIDeviceAddressPtr devAddr)
{
size_t i;
for (i = 0; i < list->count; i++) {
virPCIDevicePtr other = list->devs[i];
-if (other->address.domain == dev->address.domain &&
-other->address.bus == dev->address.bus&&
-other->address.slot == dev->address.slot &&
-other->address.function == dev->address.function)
+if (other->address.domain == devAddr->domain &&
+other->address.bus == devAddr->bus&&
+other->address.slot == devAddr->slot &&
+other->address.function == devAddr->function)
return i;
}
return -1;
@@ -1798,7 +1799,7 @@ virPCIDeviceListFind(virPCIDeviceListPtr list,
virPCIDevicePtr dev)
{
int idx;
-if ((idx = virPCIDeviceListFindIndex(list, dev)) >= 0)
+if ((idx = virPCIDeviceListFindIndex(list, >address)) >= 0)
return list->devs[idx];
else
return NULL;
diff --git a/src/util/virpci.h b/src/util/virpci.h
index a9c597a428..8c6776da21 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -175,7 +175,7 @@ virPCIDeviceListFindByIDs(virPCIDeviceListPtr list,
unsigned int slot,
unsigned int function);
int virPCIDeviceListFindIndex(virPCIDeviceListPtr list,
- virPCIDevicePtr dev);
+ virPCIDeviceAddressPtr devAddr);
/*
* Callback that will be invoked once for each file
Re: [PATCH RESEND 15/20] qemu_cgroup.c: skip absent PCI devices in qemuTeardownHostdevCgroup()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
There is no need to bother with cgroup tearing down for absent
PCI devices, given that their entries in the sysfs are already
gone.
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_cgroup.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f7146a71c9..050df21d87 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -467,6 +467,16 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
return 0;
+/* Skip tearing down Cgroup for hostdevs that represents absent
+ * PCI devices, e.g. SR-IOV virtual functions that were removed from
+ * the host while the domain was still running. */
+if (virHostdevIsPCIDevice(dev)) {
+const virDomainHostdevSubsysPCI *pcisrc = >source.subsys.u.pci;
+
+if (!virPCIDeviceExists(>addr))
+return 0;
I would have skipped creating the temprorary variable, since it's only
used once, but.. eh. Potato, potahtoe.
Reviewed-by: Laine Stump
+}
+
if (qemuDomainGetHostdevPath(dev, , NULL) < 0)
return -1;
Re: [PATCH RESEND 14/20] virhostdev.c: add virHostdevIsPCIDevice() helper
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote: Add a helper to quickly determine if a hostdev is a PCI device, instead of doing a tedius 'if' check with hostdev mode and s/tedius/tedious/ subsys type. Signed-off-by: Daniel Henrique Barboza Reviewed-by: Laine Stump
Re: [PATCH RESEND 13/20] virsh-domain.c: modernize cmdDetachDevice()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote: Use g_auto* pointers to avoid the need of a cleanup label. The type of the pointer 'virDomainPtr dom' was changed to its alias 'virshDomainPtr' to allow the use of g_autoptr(). Signed-off-by: Daniel Henrique Barboza Reviewed-by: Laine Stump
Re: [PATCH RESEND 11/20] qemu_driver.c: modernize qemuNodeDeviceReAttach()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
Add virObjectUnref an autoptr cleanup func for virNodeDevice,
then remove all unref and free calls from qemuNodeDeviceReAttach().
Signed-off-by: Daniel Henrique Barboza
---
src/datatypes.h| 2 ++
src/qemu/qemu_driver.c | 32
2 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/datatypes.h b/src/datatypes.h
index ade3779e43..7a88aba0df 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -707,6 +707,8 @@ struct _virNodeDevice {
char *parentName; /* parent device name */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNodeDevice, virObjectUnref);
It may seem like overkill, but I think this ^^^ should be added in a
separate patch. That way if some other patch that uses
g_autoptr(virNodeDevice) needs to be backported to a downstream release
that doesn't want to take the rest of this patch's refactoring of
qemuNodeDeviceReAttach(), they can do it.
Reviewed-by: Laine Stump
but split the above line into a separate patch (which you can also put
my R-b on)
+
/**
* _virSecret:
*
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7581e3c8cb..f9aa93fa3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12045,17 +12045,16 @@ static int
qemuNodeDeviceReAttach(virNodeDevicePtr dev)
{
virQEMUDriverPtr driver = dev->conn->privateData;
-virPCIDevicePtr pci = NULL;
+g_autoptr(virPCIDevice) pci = NULL;
virPCIDeviceAddress devAddr;
-int ret = -1;
-virNodeDeviceDefPtr def = NULL;
+g_autoptr(virNodeDeviceDef) def = NULL;
g_autofree char *xml = NULL;
virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
-virConnectPtr nodeconn = NULL;
-virNodeDevicePtr nodedev = NULL;
+g_autoptr(virConnect) nodeconn = NULL;
+g_autoptr(virNodeDevice) nodedev = NULL;
if (!(nodeconn = virGetConnectNodeDev()))
-goto cleanup;
+return -1;
/* 'dev' is associated with the QEMU virConnectPtr,
* so for split daemons, we need to get a copy that
@@ -12063,36 +12062,29 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
*/
if (!(nodedev = virNodeDeviceLookupByName(
nodeconn, virNodeDeviceGetName(dev
-goto cleanup;
+return -1;
xml = virNodeDeviceGetXMLDesc(nodedev, 0);
if (!xml)
-goto cleanup;
+return -1;
def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
if (!def)
-goto cleanup;
+return -1;
/* ACL check must happen against original 'dev',
* not the new 'nodedev' we acquired */
if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0)
-goto cleanup;
+return -1;
if (virDomainDriverNodeDeviceGetPCIInfo(def, ) < 0)
-goto cleanup;
+return -1;
pci = virPCIDeviceNew();
if (!pci)
-goto cleanup;
-
-ret = virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci);
+return -1;
-virPCIDeviceFree(pci);
- cleanup:
-virNodeDeviceDefFree(def);
-virObjectUnref(nodedev);
-virObjectUnref(nodeconn);
-return ret;
+return virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci);
}
static int
Re: [PATCH RESEND 10/20] libvirt-nodedev.c: remove return value from virNodeDeviceFree()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
The function returns -1 on error, but no caller is actually checking
the return value. Making it 'void' makes more sense with its
current use.
Signed-off-by: Daniel Henrique Barboza
NAK - you can't change a public function.
---
include/libvirt/libvirt-nodedev.h | 2 +-
src/libvirt-nodedev.c | 15 +++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/include/libvirt/libvirt-nodedev.h
b/include/libvirt/libvirt-nodedev.h
index eab8abf6ab..5634980a75 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -114,7 +114,7 @@ char * virNodeDeviceGetXMLDesc
(virNodeDevicePtr dev,
unsigned int flags);
int virNodeDeviceRef(virNodeDevicePtr dev);
-int virNodeDeviceFree (virNodeDevicePtr dev);
+voidvirNodeDeviceFree (virNodeDevicePtr dev);
int virNodeDeviceDettach(virNodeDevicePtr dev);
int virNodeDeviceDetachFlags(virNodeDevicePtr dev,
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index eb8c735a8c..fcca40f47b 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -445,19 +445,26 @@ virNodeDeviceListCaps(virNodeDevicePtr dev,
* Drops a reference to the node device, freeing it if
* this was the last reference.
*
- * Returns the 0 for success, -1 for error.
+ * Throws a VIR_ERR_INVALID_NODE_DEVICE error if @dev is
+ * not a valid node device. Does nothing if @dev is
+ * NULL.
*/
-int
+void
virNodeDeviceFree(virNodeDevicePtr dev)
{
+if (!dev)
+return;
+
VIR_DEBUG("dev=%p, conn=%p", dev, dev ? dev->conn : NULL);
virResetLastError();
-virCheckNodeDeviceReturn(dev, -1);
+virCheckNodeDeviceGoto(dev, invalid_device);
virObjectUnref(dev);
-return 0;
+
+ invalid_device:
+return;
}
Re: [PATCH RESEND 09/20] dac, selinux: skip setting/restoring label for absent PCI devices
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
If the underlying PCI device of a hostdev does not exist in the
host (e.g. a SR-IOV VF that was removed while the domain was
running), skip security label handling for it.
This will avoid errors that happens during qemuProcessStop() time,
where a VF that was being used by the domain is not present anymore.
The restore label functions of both DAC and SELinux drivers will
trigger errors in virPCIDeviceNew().
Signed-off-by: Daniel Henrique Barboza
This is fine as far as it goes, but what about
AppArmorSetSecurityHostdevLabel() (and also whatever it is that's going
on in the get_files() function in virt-aa-helper.c). You likely don't
have a setup to test it, but it seems pretty straightforward to
extrapolate that if you should skip setting the selinux and DAC labels
when a device doesn't exist, you can probably do the same thing for
AppArmor.
Reviewed-by: Laine Stump
but add another patch that fixes the problem for AppArmor too.
(Also, all the code repetition here makes me think that there must be a
better way to do this and reduce all the boilerplate, but I think it
would be better to just make these changes and then look at eliminating
the boilerplate afterwards, rather than re-structuring the code (which
could create regressions) while adding this new concept on top of it.
---
src/security/security_dac.c | 14 --
src/security/security_selinux.c | 14 --
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0085982bb1..a2528aeb2d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1266,7 +1266,12 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
-g_autoptr(virPCIDevice) pci = virPCIDeviceNew(>addr);
+g_autoptr(virPCIDevice) pci = NULL;
+
+if (!virPCIDeviceExists(>addr))
+break;
+
+pci = virPCIDeviceNew(>addr);
if (!pci)
return -1;
@@ -1422,7 +1427,12 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr
mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
-g_autoptr(virPCIDevice) pci = virPCIDeviceNew(>addr);
+g_autoptr(virPCIDevice) pci = NULL;
+
+if (!virPCIDeviceExists(>addr))
+break;
+
+pci = virPCIDeviceNew(>addr);
if (!pci)
return -1;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 99adf08a15..c018c0708a 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2101,7 +2101,12 @@
virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
-g_autoptr(virPCIDevice) pci = virPCIDeviceNew(>addr);
+g_autoptr(virPCIDevice) pci = NULL;
+
+if (!virPCIDeviceExists(>addr))
+break;
+
+pci = virPCIDeviceNew(>addr);
if (!pci)
return -1;
@@ -2329,7 +2334,12 @@
virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
-g_autoptr(virPCIDevice) pci = virPCIDeviceNew(>addr);
+g_autoptr(virPCIDevice) pci = NULL;
+
+if (!virPCIDeviceExists(>addr))
+break;
+
+pci = virPCIDeviceNew(>addr);
if (!pci)
return -1;
Re: [PATCH RESEND 06/20] virhostdev.c: virHostdevGetPCIHostDevice() now reports missing device
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote: Gitlab issue #72 [1] reports that removing SR-IOVs VFs before removing the devices from the running domains can have strange consequences. QEMU might be able to hotunplug the device inside the guest, but Libvirt will not be aware of that, and then the guest is now inconsistent with the domain definition. There's also the possibility of the VFs removal not succeeding while the domain is running but then, as soon as the domain is shutdown, all the VFs are removed. Libvirt can't handle the removal of the PCI devices while trying to reattach the hostdevs, and the Libvirt daemon can be left in an inconsistent state (see [2]). This patch starts to address the issue related in Gitlab #72, most notably the issue described in [2]. When shutting down a domain with SR-IOV hostdevs that got missing, virHostdevReAttachPCIDevices() is failing the whole process and failing to reattach all the PCI devices, including the ones that aren't related to the VFs that went missing. Let's make it more resilient with host changes by changing virHostdevGetPCIHostDevice() to return an exclusive error code '-2' for this case. virHostdevGetPCIHostDeviceList() can then tell when virHostdevGetPCIHostDevice() failed to find the PCI device of a hostdev and continue to make the list of PCI devices. virHostdevReAttachPCIDevices() will now be able to proceed reattaching all other valid PCI devices, at least. The 'ghost hostdevs' will be handled later on. [1] https://gitlab.com/libvirt/libvirt/-/issues/72 [2] https://gitlab.com/libvirt/libvirt/-/issues/72#note_459032148 Signed-off-by: Daniel Henrique Barboza --- src/hypervisor/virhostdev.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index bd35397f2c..dbba36193b 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -220,7 +220,8 @@ virHostdevManagerGetDefault(void) * is returned. * * Returns: 0 on success (@pci might be NULL though), - * -1 otherwise (with error reported). + * -1 otherwise (with error reported), + * -2 PCI device not found. @pci will be NULL */ static int virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev, @@ -235,6 +236,9 @@ virHostdevGetPCIHostDevice(const virDomainHostdevDef *hostdev, hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) return 0; +if (!virPCIDeviceExists(>addr)) +return -2; You've created a special return code to mean "This is a PCI device, but it isn't present on the host"... + actual = virPCIDeviceNew(>addr); if (!actual) @@ -270,7 +274,7 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) virDomainHostdevDefPtr hostdev = hostdevs[i]; g_autoptr(virPCIDevice) pci = NULL; -if (virHostdevGetPCIHostDevice(hostdev, ) < 0) +if (virHostdevGetPCIHostDevice(hostdev, ) == -1) return NULL; ...But you haven't actually used it. Will it become necessary later to have this special value? right now a missing device is treated exactly the same as if it isn't a PCI device. I guess I can understand the conceptual desire to return an error of some type in this case though, and there are other places where something similar is done (-2 indicates some type of "odd" error), so I'll let it pass :-) Reviewed-by: Laine Stump if (!pci)
Re: [PATCH RESEND 12/20] libxl_driver.c: modernize libxlNodeDeviceReAttach()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
Use g_auto* wherever we can and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza
This patch is also obsoleted by 23cdab6a3de0f6336505adcb446f77a6e0628e6b
---
src/libxl/libxl_driver.c | 37 ++---
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3eaf106006..fbb67e9ed6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5852,18 +5852,17 @@ libxlNodeDeviceDettach(virNodeDevicePtr dev)
static int
libxlNodeDeviceReAttach(virNodeDevicePtr dev)
{
-virPCIDevicePtr pci = NULL;
+g_autoptr(virPCIDevice) pci = NULL;
virPCIDeviceAddress devAddr;
-int ret = -1;
-virNodeDeviceDefPtr def = NULL;
-char *xml = NULL;
+g_autoptr(virNodeDeviceDef) def = NULL;
+g_autofree char *xml = NULL;
libxlDriverPrivatePtr driver = dev->conn->privateData;
virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
-virConnectPtr nodeconn = NULL;
-virNodeDevicePtr nodedev = NULL;
+g_autoptr(virConnect) nodeconn = NULL;
+g_autoptr(virNodeDevice) nodedev = NULL;
if (!(nodeconn = virGetConnectNodeDev()))
-goto cleanup;
+return -1;
/* 'dev' is associated with the QEMU virConnectPtr,
* so for split daemons, we need to get a copy that
@@ -5871,40 +5870,32 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev)
*/
if (!(nodedev = virNodeDeviceLookupByName(
nodeconn, virNodeDeviceGetName(dev
-goto cleanup;
+return -1;
xml = virNodeDeviceGetXMLDesc(nodedev, 0);
if (!xml)
-goto cleanup;
+return -1;
def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
if (!def)
-goto cleanup;
+return -1;
/* ACL check must happen against original 'dev',
* not the new 'nodedev' we acquired */
if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0)
-goto cleanup;
+return -1;
if (virDomainDriverNodeDeviceGetPCIInfo(def, ) < 0)
-goto cleanup;
+return -1;
pci = virPCIDeviceNew();
if (!pci)
-goto cleanup;
+return -1;
if (virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;
- cleanup:
-virPCIDeviceFree(pci);
-virNodeDeviceDefFree(def);
-virObjectUnref(nodedev);
-virObjectUnref(nodeconn);
-VIR_FREE(xml);
-return ret;
+return 0;
}
static int
Re: [PATCH RESEND 11/20] qemu_driver.c: modernize qemuNodeDeviceReAttach()
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
Add virObjectUnref an autoptr cleanup func for virNodeDevice,
then remove all unref and free calls from qemuNodeDeviceReAttach().
Signed-off-by: Daniel Henrique Barboza
This patch is obsoleted by your own commit
23cdab6a3de0f6336505adcb446f77a6e0628e6b
---
src/datatypes.h| 2 ++
src/qemu/qemu_driver.c | 32
2 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/datatypes.h b/src/datatypes.h
index ade3779e43..7a88aba0df 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -707,6 +707,8 @@ struct _virNodeDevice {
char *parentName; /* parent device name */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNodeDevice, virObjectUnref);
+
/**
* _virSecret:
*
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7581e3c8cb..f9aa93fa3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12045,17 +12045,16 @@ static int
qemuNodeDeviceReAttach(virNodeDevicePtr dev)
{
virQEMUDriverPtr driver = dev->conn->privateData;
-virPCIDevicePtr pci = NULL;
+g_autoptr(virPCIDevice) pci = NULL;
virPCIDeviceAddress devAddr;
-int ret = -1;
-virNodeDeviceDefPtr def = NULL;
+g_autoptr(virNodeDeviceDef) def = NULL;
g_autofree char *xml = NULL;
virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
-virConnectPtr nodeconn = NULL;
-virNodeDevicePtr nodedev = NULL;
+g_autoptr(virConnect) nodeconn = NULL;
+g_autoptr(virNodeDevice) nodedev = NULL;
if (!(nodeconn = virGetConnectNodeDev()))
-goto cleanup;
+return -1;
/* 'dev' is associated with the QEMU virConnectPtr,
* so for split daemons, we need to get a copy that
@@ -12063,36 +12062,29 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
*/
if (!(nodedev = virNodeDeviceLookupByName(
nodeconn, virNodeDeviceGetName(dev
-goto cleanup;
+return -1;
xml = virNodeDeviceGetXMLDesc(nodedev, 0);
if (!xml)
-goto cleanup;
+return -1;
def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
if (!def)
-goto cleanup;
+return -1;
/* ACL check must happen against original 'dev',
* not the new 'nodedev' we acquired */
if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0)
-goto cleanup;
+return -1;
if (virDomainDriverNodeDeviceGetPCIInfo(def, ) < 0)
-goto cleanup;
+return -1;
pci = virPCIDeviceNew();
if (!pci)
-goto cleanup;
-
-ret = virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci);
+return -1;
-virPCIDeviceFree(pci);
- cleanup:
-virNodeDeviceDefFree(def);
-virObjectUnref(nodedev);
-virObjectUnref(nodeconn);
-return ret;
+return virHostdevPCINodeDeviceReAttach(hostdev_mgr, pci);
}
static int
Re: [PATCH RESEND 00/20] handle missing SR-IOV VF hostdevs during running domains
On 2/16/21 8:16 AM, Daniel Henrique Barboza wrote: Ping for reviews (patches 1-4 and 7-8 already pushed). Yeah, sorry. It's been on my list ever since I left it half-finished, but I keep getting distra... OOHH LOOK!!! SOMETHING SHINY!! Okay, back to the subject - I promise I will go back to these before today is finished!!
[libvirt PATCH 6/9] esx: reorder code to avoid need to VIR_FREE mimeType
mimeType is initialized to NULL, and then only set in one place, just
before a check (not involving mimeType) that then VIR_FREEs mimeType
if it fails. If we just reorder the code to do the check prior to
setting mimeType, then there won't be any need to VIR_FREE(mimeType)
on failure (because it will already be empty/NULL).
Signed-off-by: Laine Stump
---
src/esx/esx_driver.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 47873c0d54..2d010096a5 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2374,12 +2374,10 @@ esxDomainScreenshot(virDomainPtr domain, virStreamPtr
stream,
url = virBufferContentAndReset();
-mimeType = g_strdup("image/png");
-
-if (esxStreamOpenDownload(stream, priv, url, 0, 0) < 0) {
-VIR_FREE(mimeType);
+if (esxStreamOpenDownload(stream, priv, url, 0, 0) < 0)
goto cleanup;
-}
+
+mimeType = g_strdup("image/png");
cleanup:
--
2.29.2
[libvirt PATCH 8/9] esx: eliminate unnecessary cleanup: labels and result variables
switching to g_autofree left many cleanup: sections empty.
Signed-off-by: Laine Stump
---
src/esx/esx_driver.c | 22 ++-
src/esx/esx_storage_backend_vmfs.c | 22 +--
src/esx/esx_util.c | 8 ++
src/esx/esx_vi.c | 45 +-
src/esx/esx_vi_types.c | 8 ++
5 files changed, 38 insertions(+), 67 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index fe98f90ea9..e49975de86 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -690,7 +690,6 @@ esxConnectToVCenter(esxPrivate *priv,
const char *hostname,
const char *hostSystemIPAddress)
{
-int result = -1;
g_autofree char *ipAddress = NULL;
g_autofree char *username = NULL;
g_autofree char *password = NULL;
@@ -711,11 +710,11 @@ esxConnectToVCenter(esxPrivate *priv,
} else {
if (!(username = virAuthGetUsername(conn, auth, "esx", "administrator",
hostname)))
-goto cleanup;
+return -1;
}
if (!(password = virAuthGetPassword(conn, auth, "esx", username,
hostname)))
-goto cleanup;
+return -1;
url = g_strdup_printf("%s://%s:%d/sdk", priv->parsedUri->transport,
hostname,
conn->uri->port);
@@ -723,7 +722,7 @@ esxConnectToVCenter(esxPrivate *priv,
if (esxVI_Context_Alloc(>vCenter) < 0 ||
esxVI_Context_Connect(priv->vCenter, url, ipAddress, username,
password, priv->parsedUri) < 0) {
-goto cleanup;
+return -1;
}
if (priv->vCenter->productLine != esxVI_ProductLine_VPX) {
@@ -732,25 +731,20 @@ esxConnectToVCenter(esxPrivate *priv,
hostname,
esxVI_ProductLineToDisplayName(esxVI_ProductLine_VPX),
esxVI_ProductLineToDisplayName(priv->vCenter->productLine));
-goto cleanup;
+return -1;
}
if (hostSystemIPAddress) {
-if (esxVI_Context_LookupManagedObjectsByHostSystemIp
- (priv->vCenter, hostSystemIPAddress) < 0) {
-goto cleanup;
-}
+if (esxVI_Context_LookupManagedObjectsByHostSystemIp(priv->vCenter,
hostSystemIPAddress) < 0)
+return -1;
} else {
if (esxVI_Context_LookupManagedObjectsByPath(priv->vCenter,
priv->parsedUri->path) <
0) {
-goto cleanup;
+return -1;
}
}
-result = 0;
-
- cleanup:
-return result;
+return 0;
}
diff --git a/src/esx/esx_storage_backend_vmfs.c
b/src/esx/esx_storage_backend_vmfs.c
index 95505d6796..cb2be59a33 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -667,7 +667,6 @@ static virStorageVolPtr
esxStorageVolLookupByName(virStoragePoolPtr pool,
const char *name)
{
-virStorageVolPtr volume = NULL;
esxPrivate *priv = pool->conn->privateData;
g_autofree char *datastorePath = NULL;
g_autofree char *key = NULL;
@@ -676,14 +675,11 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary,
datastorePath, ) < 0) {
-goto cleanup;
+return NULL;
}
-volume = virGetStorageVol(pool->conn, pool->name, name, key,
- , NULL);
-
- cleanup:
-return volume;
+return virGetStorageVol(pool->conn, pool->name, name, key,
+, NULL);
}
@@ -691,7 +687,6 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
static virStorageVolPtr
esxStorageVolLookupByPath(virConnectPtr conn, const char *path)
{
-virStorageVolPtr volume = NULL;
esxPrivate *priv = conn->privateData;
g_autofree char *datastoreName = NULL;
g_autofree char *directoryAndFileName = NULL;
@@ -699,19 +694,16 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char
*path)
if (esxUtil_ParseDatastorePath(path, , NULL,
) < 0) {
-goto cleanup;
+return NULL;
}
if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path,
) < 0) {
-goto cleanup;
+return NULL;
}
-volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, key,
- , NULL);
-
- cleanup:
-return volume;
+return virGetStorageVol(conn, datastoreName, directoryAndFileName, key,
+, NULL);
}
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index f1e8339fe0..ef070a4f04 1006
[libvirt PATCH 9/9] esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
These are all cases when 1) the pointer is passed by reference from
the caller (ie.e. **) and expects it to be NULL on return if there is
an error, or 2) the variable holding the pointer is being checked or
re-used in the same function, but not right away.
Signed-off-by: Laine Stump
---
src/esx/esx_network_driver.c | 2 +-
src/esx/esx_util.c | 8
src/esx/esx_vi.c | 4 ++--
src/esx/esx_vi_types.c | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index b489f4de8a..4d0fba8c9f 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -914,7 +914,7 @@ esxConnectListAllNetworks(virConnectPtr conn,
if (nets && *nets) {
for (i = 0; i < count; ++i)
g_free((*nets)[i]);
-VIR_FREE(*nets);
+g_clear_pointer(nets, g_free);
}
}
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index ef070a4f04..24e1c73ec4 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -95,7 +95,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri)
/* Expected format: [://][:] */
(*parsedUri)->proxy = true;
(*parsedUri)->proxy_type = CURLPROXY_HTTP;
-VIR_FREE((*parsedUri)->proxy_hostname);
+g_clear_pointer(&(*parsedUri)->proxy_hostname, g_free);
(*parsedUri)->proxy_port = 1080;
if ((tmp = STRSKIP(queryParam->value, "http://;))) {
@@ -261,13 +261,13 @@ esxUtil_ParseDatastorePath(const char *datastorePath,
char **datastoreName,
cleanup:
if (result < 0) {
if (datastoreName)
-VIR_FREE(*datastoreName);
+g_clear_pointer(datastoreName, g_free);
if (directoryName)
-VIR_FREE(*directoryName);
+g_clear_pointer(directoryName, g_free);
if (directoryAndFileName)
-VIR_FREE(*directoryAndFileName);
+g_clear_pointer(directoryAndFileName, g_free);
}
return result;
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index db5035c035..e1c1a15ab6 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -73,7 +73,7 @@ VIR_LOG_INIT("esx.esx_vi");
\
_body \
\
-VIR_FREE(*ptrptr); \
+g_clear_pointer(ptrptr, g_free); \
}
@@ -2516,7 +2516,7 @@ esxVI_GetVirtualMachineIdentity(esxVI_ObjectContent
*virtualMachine,
failure:
if (name)
-VIR_FREE(*name);
+g_clear_pointer(name, g_free);
return -1;
}
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 4dc7c30680..59735194ae 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -67,7 +67,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
\
_body \
\
-VIR_FREE(*ptrptr); \
+g_clear_pointer(ptrptr, g_free); \
}
--
2.29.2
[libvirt PATCH 1/9] esx: use g_autofree for char* where it is trivially possible
All of these strings are allocated once, freed once, and are never
returned out of the function where they are created, used, and are
freed.
Signed-off-by: Laine Stump
---
src/esx/esx_driver.c | 128 +
src/esx/esx_storage_backend_vmfs.c | 102 ---
src/esx/esx_stream.c | 7 +-
src/esx/esx_util.c | 11 +--
src/esx/esx_vi.c | 53
src/esx/esx_vi_methods.c | 3 +-
src/esx/esx_vi_types.c | 8 +-
7 files changed, 93 insertions(+), 219 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 0271f81a56..df257341b8 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -139,8 +139,8 @@ esxParseVMXFileName(const char *fileName,
char *datastoreName;
char *tmp;
char *saveptr;
-char *strippedFileName = NULL;
-char *copyOfFileName = NULL;
+g_autofree char *strippedFileName = NULL;
+g_autofree char *copyOfFileName = NULL;
char *directoryAndFileName;
int ret = -1;
@@ -253,8 +253,6 @@ esxParseVMXFileName(const char *fileName,
esxVI_String_Free();
esxVI_ObjectContent_Free();
esxVI_DatastoreHostMount_Free();
-VIR_FREE(strippedFileName);
-VIR_FREE(copyOfFileName);
return ret;
}
@@ -280,8 +278,8 @@ esxFormatVMXFileName(const char *fileName, void *opaque)
bool success = false;
char *result = NULL;
esxVMX_Data *data = opaque;
-char *datastoreName = NULL;
-char *directoryAndFileName = NULL;
+g_autofree char *datastoreName = NULL;
+g_autofree char *directoryAndFileName = NULL;
esxVI_ObjectContent *datastore = NULL;
esxVI_DatastoreHostMount *hostMount = NULL;
char separator = '/';
@@ -349,8 +347,6 @@ esxFormatVMXFileName(const char *fileName, void *opaque)
if (! success)
VIR_FREE(result);
-VIR_FREE(datastoreName);
-VIR_FREE(directoryAndFileName);
esxVI_ObjectContent_Free();
esxVI_DatastoreHostMount_Free();
@@ -613,9 +609,9 @@ esxConnectToHost(esxPrivate *priv,
{
int result = -1;
g_autofree char *ipAddress = NULL;
-char *username = NULL;
-char *password = NULL;
-char *url = NULL;
+g_autofree char *username = NULL;
+g_autofree char *password = NULL;
+g_autofree char *url = NULL;
esxVI_String *propertyNameList = NULL;
esxVI_ObjectContent *hostSystem = NULL;
esxVI_Boolean inMaintenanceMode = esxVI_Boolean_Undefined;
@@ -683,9 +679,6 @@ esxConnectToHost(esxPrivate *priv,
result = 0;
cleanup:
-VIR_FREE(username);
-VIR_FREE(password);
-VIR_FREE(url);
esxVI_String_Free();
esxVI_ObjectContent_Free();
@@ -703,9 +696,9 @@ esxConnectToVCenter(esxPrivate *priv,
{
int result = -1;
g_autofree char *ipAddress = NULL;
-char *username = NULL;
-char *password = NULL;
-char *url = NULL;
+g_autofree char *username = NULL;
+g_autofree char *password = NULL;
+g_autofree char *url = NULL;
if (!hostSystemIPAddress &&
(!priv->parsedUri->path || STREQ(priv->parsedUri->path, "/"))) {
@@ -761,10 +754,6 @@ esxConnectToVCenter(esxPrivate *priv,
result = 0;
cleanup:
-VIR_FREE(username);
-VIR_FREE(password);
-VIR_FREE(url);
-
return result;
}
@@ -822,7 +811,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
{
virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
esxPrivate *priv = NULL;
-char *potentialVCenterIPAddress = NULL;
+g_autofree char *potentialVCenterIPAddress = NULL;
g_autofree char *vCenterIPAddress = NULL;
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
@@ -938,8 +927,6 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
cleanup:
esxFreePrivate();
-VIR_FREE(potentialVCenterIPAddress);
-
return result;
}
@@ -1472,7 +1459,7 @@ esxDomainLookupByUUID(virConnectPtr conn, const unsigned
char *uuid)
esxVI_ObjectContent *virtualMachine = NULL;
esxVI_VirtualMachinePowerState powerState;
int id = -1;
-char *name = NULL;
+g_autofree char *name = NULL;
virDomainPtr domain = NULL;
if (esxVI_EnsureSession(priv->primary) < 0)
@@ -1498,8 +1485,6 @@ esxDomainLookupByUUID(virConnectPtr conn, const unsigned
char *uuid)
cleanup:
esxVI_String_Free();
esxVI_ObjectContent_Free();
-VIR_FREE(name);
-
return domain;
}
@@ -1559,7 +1544,7 @@ esxDomainSuspend(virDomainPtr domain)
esxVI_VirtualMachinePowerState powerState;
esxVI_ManagedObjectReference *task = NULL;
esxVI_TaskInfoState taskInfoState;
-char *taskInfoErrorMessage = NULL;
+g_autofree char *taskInfoErrorMessage = NULL;
if (esxVI_EnsureSession(priv->primary) < 0)
return -1;
@@ -1599,8 +1584,6 @@ esxDomainSuspend(virDomainPtr domain)
esxVI_ObjectContent_Free();
esxVI_String_Free();
esxVI_Ma
[libvirt PATCH 4/9] esx: switch VIR_FREE->g_free in esx*Free*()
Although the three functions esxFreePrivate(), esxFreeStreamPrivate(), and esxUtil_FreeParsedUri() are calling VIR_FREE on *object, and so in theory the caller of the function might rely on "object" (the free function's arg) being set to NULL, in practice these functions are only called from a couple places each, and in all cases the pointer that is passed is a local variable, and goes out of scope almost immediately after calling the Free function, so it is safe to change VIR_FREE() into g_free(). Signed-off-by: Laine Stump --- src/esx/esx_driver.c | 2 +- src/esx/esx_stream.c | 4 ++-- src/esx/esx_util.c | 10 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 8afec464dd..952e769376 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -71,7 +71,7 @@ esxFreePrivate(esxPrivate **priv) esxUtil_FreeParsedUri(&(*priv)->parsedUri); virObjectUnref((*priv)->caps); virObjectUnref((*priv)->xmlopt); -VIR_FREE(*priv); +g_free(*priv); } diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c index cc48c182d9..131fbc100b 100644 --- a/src/esx/esx_stream.c +++ b/src/esx/esx_stream.c @@ -336,8 +336,8 @@ esxFreeStreamPrivate(esxStreamPrivate **priv) return; esxVI_CURL_Free(&(*priv)->curl); -VIR_FREE((*priv)->backlog); -VIR_FREE(*priv); +g_free((*priv)->backlog); +g_free(*priv); } static int diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 64a2c968f0..e9b74f386f 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -171,12 +171,12 @@ esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri) if (!parsedUri || !(*parsedUri)) return; -VIR_FREE((*parsedUri)->transport); -VIR_FREE((*parsedUri)->vCenter); -VIR_FREE((*parsedUri)->proxy_hostname); -VIR_FREE((*parsedUri)->path); +g_free((*parsedUri)->transport); +g_free((*parsedUri)->vCenter); +g_free((*parsedUri)->proxy_hostname); +g_free((*parsedUri)->path); -VIR_FREE(*parsedUri); +g_free(*parsedUri); } -- 2.29.2
[libvirt PATCH 7/9] esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope
Or when it will be immediately have a new value assigned to it.
Signed-off-by: Laine Stump
---
src/esx/esx_driver.c| 4 ++--
src/esx/esx_network_driver.c| 2 +-
src/esx/esx_storage_backend_iscsi.c | 4 ++--
src/esx/esx_storage_backend_vmfs.c | 4 ++--
src/esx/esx_util.c | 4 ++--
src/esx/esx_vi.c| 4 ++--
6 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 2d010096a5..fe98f90ea9 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2619,7 +2619,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int
flags)
cleanup:
esxVI_String_Free();
esxVI_ObjectContent_Free();
-VIR_FREE(data.datastorePathWithoutFileName);
+g_free(data.datastorePathWithoutFileName);
virDomainDefFree(def);
return xml;
@@ -4946,7 +4946,7 @@ esxConnectListAllDomains(virConnectPtr conn,
for (id = 0; id < count; id++)
virObjectUnref(doms[id]);
-VIR_FREE(doms);
+g_free(doms);
}
esxVI_AutoStartDefaults_Free();
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 15fc7931c0..b489f4de8a 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -913,7 +913,7 @@ esxConnectListAllNetworks(virConnectPtr conn,
if (ret < 0) {
if (nets && *nets) {
for (i = 0; i < count; ++i)
-VIR_FREE((*nets)[i]);
+g_free((*nets)[i]);
VIR_FREE(*nets);
}
}
diff --git a/src/esx/esx_storage_backend_iscsi.c
b/src/esx/esx_storage_backend_iscsi.c
index 41bb9f6094..d89b5a4ba8 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -348,7 +348,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned
int flags)
xml = virStoragePoolDefFormat();
cleanup:
-VIR_FREE(def.source.hosts);
+g_free(def.source.hosts);
esxVI_HostInternetScsiHba_Free();
return xml;
@@ -727,7 +727,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume,
cleanup:
esxVI_ScsiLun_Free();
-VIR_FREE(def.key);
+g_free(def.key);
return xml;
}
diff --git a/src/esx/esx_storage_backend_vmfs.c
b/src/esx/esx_storage_backend_vmfs.c
index 225b2a4751..95505d6796 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -544,7 +544,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned
int flags)
xml = virStoragePoolDefFormat();
cleanup:
-VIR_FREE(def.source.hosts);
+g_free(def.source.hosts);
esxVI_String_Free();
esxVI_ObjectContent_Free();
esxVI_DatastoreHostMount_Free();
@@ -1390,7 +1390,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume,
cleanup:
esxVI_FileInfo_Free();
-VIR_FREE(def.key);
+g_free(def.key);
return xml;
}
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index e9b74f386f..f1e8339fe0 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -55,7 +55,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri)
virURIParamPtr queryParam = >params[i];
if (STRCASEEQ(queryParam->name, "transport")) {
-VIR_FREE((*parsedUri)->transport);
+g_free((*parsedUri)->transport);
(*parsedUri)->transport = g_strdup(queryParam->value);
@@ -68,7 +68,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri)
goto cleanup;
}
} else if (STRCASEEQ(queryParam->name, "vcenter")) {
-VIR_FREE((*parsedUri)->vCenter);
+g_free((*parsedUri)->vCenter);
(*parsedUri)->vCenter = g_strdup(queryParam->value);
} else if (STRCASEEQ(queryParam->name, "no_verify")) {
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 987259be4b..2a999f1cc1 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -4280,7 +4280,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx,
if (esxVI_WaitForUpdates(ctx, version, ) < 0)
goto cleanup;
-VIR_FREE(version);
+g_free(version);
version = g_strdup(updateSet->version);
if (!updateSet->filterSet)
@@ -4355,7 +4355,7 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx,
esxVI_PropertyFilterSpec_Free();
esxVI_ManagedObjectReference_Free();
-VIR_FREE(version);
+g_free(version);
esxVI_UpdateSet_Free();
esxVI_TaskInfo_Free();
--
2.29.2
[libvirt PATCH 5/9] esx: use g_steal_pointer+g_autofree on return value
If we put the potential return string into the g_autofreed tmpResult,
and the move it to the returned "result" only as a final step ater, we
can avoid the need to explicitly VIR_FREE (or g_free) on failure.
Signed-off-by: Laine Stump
---
src/esx/esx_driver.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 952e769376..47873c0d54 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -275,7 +275,7 @@ esxParseVMXFileName(const char *fileName,
static char *
esxFormatVMXFileName(const char *fileName, void *opaque)
{
-bool success = false;
+g_autofree char *tmpResult = NULL;
char *result = NULL;
esxVMX_Data *data = opaque;
g_autofree char *datastoreName = NULL;
@@ -329,10 +329,10 @@ esxFormatVMXFileName(const char *fileName, void *opaque)
virBufferAddChar(, separator);
virBufferAdd(, directoryAndFileName, -1);
-result = virBufferContentAndReset();
+tmpResult = virBufferContentAndReset();
} else if (*fileName == '/') {
/* FIXME: need to deal with Windows paths here too */
-result = g_strdup(fileName);
+tmpResult = g_strdup(fileName);
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not handle file name '%s'"), fileName);
@@ -341,15 +341,11 @@ esxFormatVMXFileName(const char *fileName, void *opaque)
/* FIXME: Check if referenced path/file really exists */
-success = true;
+result = g_steal_pointer();
cleanup:
-if (! success)
-VIR_FREE(result);
-
esxVI_ObjectContent_Free();
esxVI_DatastoreHostMount_Free();
-
return result;
}
--
2.29.2
[libvirt PATCH 2/9] esx: use g_autofree when made possible by reducing scope
These strings were being VIR_FREEd multiple times because they were
defined at the top of a function, but then set each time through a
loop. But they are only used inside that loop, so they can be
converted to use g_autofree if their definition is also placed inside
that loop.
Signed-off-by: Laine Stump
---
src/esx/esx_driver.c| 13 -
src/esx/esx_storage_backend_iscsi.c | 12
src/esx/esx_storage_backend_vmfs.c | 18 --
src/esx/esx_vi.c| 5 +
4 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index df257341b8..8afec464dd 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1391,7 +1391,6 @@ esxDomainLookupByID(virConnectPtr conn, int id)
esxVI_ObjectContent *virtualMachine = NULL;
esxVI_VirtualMachinePowerState powerState;
int id_candidate = -1;
-char *name_candidate = NULL;
unsigned char uuid_candidate[VIR_UUID_BUFLEN];
virDomainPtr domain = NULL;
@@ -1410,6 +1409,8 @@ esxDomainLookupByID(virConnectPtr conn, int id)
for (virtualMachine = virtualMachineList; virtualMachine;
virtualMachine = virtualMachine->_next) {
+g_autofree char *name_candidate = NULL;
+
if (esxVI_GetVirtualMachinePowerState(virtualMachine,
) < 0) {
goto cleanup;
@@ -1419,8 +1420,6 @@ esxDomainLookupByID(virConnectPtr conn, int id)
if (powerState == esxVI_VirtualMachinePowerState_PoweredOff)
continue;
-VIR_FREE(name_candidate);
-
if (esxVI_GetVirtualMachineIdentity(virtualMachine,
_candidate, _candidate,
uuid_candidate) < 0) {
@@ -1444,8 +1443,6 @@ esxDomainLookupByID(virConnectPtr conn, int id)
cleanup:
esxVI_String_Free();
esxVI_ObjectContent_Free();
-VIR_FREE(name_candidate);
-
return domain;
}
@@ -4754,7 +4751,6 @@ esxConnectListAllDomains(virConnectPtr conn,
esxVI_AutoStartPowerInfo *powerInfoList = NULL;
esxVI_AutoStartPowerInfo *powerInfo = NULL;
esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL;
-char *name = NULL;
int id;
unsigned char uuid[VIR_UUID_BUFLEN];
int count = 0;
@@ -4839,9 +4835,9 @@ esxConnectListAllDomains(virConnectPtr conn,
for (virtualMachine = virtualMachineList; virtualMachine;
virtualMachine = virtualMachine->_next) {
-if (needIdentity) {
-VIR_FREE(name);
+g_autofree char *name = NULL;
+if (needIdentity) {
if (esxVI_GetVirtualMachineIdentity(virtualMachine, ,
, uuid) < 0) {
goto cleanup;
@@ -4959,7 +4955,6 @@ esxConnectListAllDomains(virConnectPtr conn,
VIR_FREE(doms);
}
-VIR_FREE(name);
esxVI_AutoStartDefaults_Free();
esxVI_AutoStartPowerInfo_Free();
esxVI_String_Free();
diff --git a/src/esx/esx_storage_backend_iscsi.c
b/src/esx/esx_storage_backend_iscsi.c
index 4d16ba2520..41bb9f6094 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -494,7 +494,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char
*path)
esxVI_ScsiLun *scsiLunList = NULL;
esxVI_ScsiLun *scsiLun;
esxVI_HostScsiDisk *hostScsiDisk = NULL;
-char *poolName = NULL;
/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
@@ -503,11 +502,12 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char
*path)
goto cleanup;
for (scsiLun = scsiLunList; scsiLun; scsiLun = scsiLun->_next) {
+g_autofree char *poolName = NULL;
+
hostScsiDisk = esxVI_HostScsiDisk_DynamicCast(scsiLun);
if (hostScsiDisk && STREQ(hostScsiDisk->devicePath, path)) {
/* Found matching device */
-VIR_FREE(poolName);
if (esxVI_LookupStoragePoolNameByScsiLunKey(priv->primary,
hostScsiDisk->key,
@@ -527,8 +527,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char
*path)
cleanup:
esxVI_ScsiLun_Free();
-VIR_FREE(poolName);
-
return volume;
}
@@ -539,7 +537,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char
*key)
{
virStorageVolPtr volume = NULL;
esxPrivate *priv = conn->privateData;
-char *poolName = NULL;
esxVI_ScsiLun *scsiLunList = NULL;
esxVI_ScsiLun *scsiLun;
/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
@@ -555,6 +552,8 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char
*key)
for (scsiLun = scsiLunList; scsiLun;
scsiLun = scsiLun->_next) {
+g_autofree char *poolName
[libvirt PATCH 0/9] eliminate (almost) all VIR_FREE in esx directory
142 more down, 3084 to go :-) This goes through all the standard methods of eliminating VIR_FREE - first converting as many pointers as possible to g_autofree, then converting a few more *Free* functions (that were more questionable than previous Frees) to use g_free, then some trivial refactoring. And finally at the end a few stragglers that really do need the pointers cleared were changed to g_clear_pointer(x, g_free). A couple of issues: 1) There are two definitions of a macro called ESX_VI__TEMPLATE__FREE that generate functions with names in the form of "esxVI_.*_Free". These generated functions were doing "VIR_FREE(*ptrptr)" at the end; because of the "*", the caller's pointer would be cleared out (not just the local copy). There are something over 400 calls to these functions, and all but 10 or so that I audited will never reference the pointer after return from esxVI_Blah_Free(). But there are several that do. Since I'm unfamiliar with this code and don't want to risk breaking it, I opted to just use g_clear_pointer(ptrptr, g_free), thus preserving current behavior exactly. 2) There are still 7 instances of VIR_FREE in the esx directory. All 7 of them are in loops that are clearing out an array of names prior to returning failure, and from a quick glance it looks like there are at least a few places where it is important to clear the array entries. But rather than brute force convert to using g_clear_pointer in the loop, I figured someone may come up with an elegant translation to use GArray or GPtrArray instead, so I'm leaving them for now. Laine Stump (9): esx: use g_autofree for char* where it is trivially possible esx: use g_autofree when made possible by reducing scope esx: fix memory leak by switching to g_autofree esx: switch VIR_FREE->g_free in esx*Free*() esx: use g_steal_pointer+g_autofree on return value esx: reorder code to avoid need to VIR_FREE mimeType esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope esx: eliminate unnecessary cleanup: labels and result variables esx: replace some VIR_FREE with g_clear_pointer(x, g_free) src/esx/esx_driver.c| 189 +--- src/esx/esx_network_driver.c| 4 +- src/esx/esx_storage_backend_iscsi.c | 16 +-- src/esx/esx_storage_backend_vmfs.c | 150 +++--- src/esx/esx_stream.c| 11 +- src/esx/esx_util.c | 41 +++--- src/esx/esx_vi.c| 111 ++-- src/esx/esx_vi_methods.c| 3 +- src/esx/esx_vi_types.c | 18 +-- 9 files changed, 179 insertions(+), 364 deletions(-) -- 2.29.2
[libvirt PATCH 3/9] esx: fix memory leak by switching to g_autofree
volumeName was defined at the top of the function, then a new string
was assigned to it each time through a loop, but after the first
iteration of the loop, the previous string wasn't freed before
allocating a new string the next time. By reducing the scope of
volumeName to be just the loop, and making it g_autofree, we eliminate
the leak.
Signed-off-by: Laine Stump
---
src/esx/esx_storage_backend_vmfs.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/esx/esx_storage_backend_vmfs.c
b/src/esx/esx_storage_backend_vmfs.c
index 63959ec237..225b2a4751 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -728,7 +728,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char
*key)
esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL;
esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL;
size_t length;
-char *volumeName = NULL;
esxVI_FileInfo *fileInfo = NULL;
char key_candidate[VIR_UUID_STRING_BUFLEN] = "";
@@ -789,6 +788,7 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char
*key)
/* Build datastore path and query the UUID */
for (fileInfo = searchResults->file; fileInfo;
fileInfo = fileInfo->_next) {
+g_autofree char *volumeName = NULL;
g_autofree char *datastorePath = NULL;
g_autofree char *uuid_string = NULL;
@@ -831,8 +831,6 @@ esxStorageVolLookupByKey(virConnectPtr conn, const char
*key)
esxVI_String_Free();
esxVI_ObjectContent_Free();
esxVI_HostDatastoreBrowserSearchResults_Free();
-VIR_FREE(volumeName);
-
return volume;
}
--
2.29.2
Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions
On 2/12/21 5:25 AM, Daniel P. Berrangé wrote: On Fri, Feb 12, 2021 at 11:07:21AM +0100, Erik Skultety wrote: On Fri, Feb 12, 2021 at 10:43:56AM +0100, Martin Kletzander wrote: On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: I've looked at a few of these, and one thing I've found is that very often we have a function called somethingSomethingClear(), and: 1) The only places it is ever called will immediately free the memory of the object as soon as they clear it. and very possibly 2) It doesn't actually *clear* everything. Some items are cleared via VIR_FREE(), but then some of the other pointers call bobLoblawFree(def->bobloblaw) and then don't actually set def->bobloblaw to NULL - so the functions aren't actually "Clearing", they're "Freeing and then clearing a few things, but not everything". One thing I am wondering is whether this is really only used where it makes sense. As far as I understand, and please correct me if I am way off, the purpose of the Clear functions is to: a) provide a way to remove everything from a structure that the current function cannot recreate (there is a pointer to it somewhere else which would not be updated) and b) provide a way to reset a structure so that it can be filled again without needless reallocation. I think (b) is obviously pointless, especially lately, so the only reasonable usage would be for the scenario (a). However, I think I remember this also being used in places where it would be perfectly fine to free the variable and recreate it. Maybe it could ease up the decision, at least by eliminating some of the code, if my hunch is correct. In my quick search I only found virDomainVideoDefClear to be used in this manner and I am not convinced that it is worth having this extra function with extra You could always memset it explicitly, someone might find the code more readable then. IMO I'd vote for an explicit memset just for the sake of better security practice (since we'll have to wait a little while for something like SGX to be convenient to deploy and develop with...). Anyhow, I'm not sure how many cycles exactly would be wasted, but IIRC a recent discussion memset can be optimized away (correct me if I don't remember it well!), so Dan P.B. suggested to gradually convert to some platform-specific ways on how to sanitize the memory safely - with that in mind, I'd say we use an explicit memset in all the functions in question and convert them later? I only suggest that for places where security is required. ie to scrub passwords. Yeah, I'm not planning to touch anything that is clearing out passwords and such. Only the *Clear() functions that currently have the dual purposes of: 1) freeing memory pointed to by the object in question (and any sub-objects) 2) clearing out the object so that it can be re-used with no side effects (e.g., pointers NULLed so that subsequent uses believe (correctly) that nothing is being pointed at, setting counters to 0, types to ..._NONE, etc. If the compiler wants to cull memsets in places unrelated to security that's fine by me, or at least, not our problem to worry about. I would hope that the compiler would be smart enough to not optimize it out if it can't determine 100% that it will never make a difference. This would mean that, for example, unless a *Clear() function is defined static, it couldn't optimize out a memset() at the end (because it can't know what would be done with the object after return). But if it's going to optimize out a memset, it would likely also optimize out the "loblaw = NULL;" in the VIR_FREE invocation, so... (My mind keeps going back to 1994, when I turned on the 80386 "invalid address faults" bit (forget the exact name) on our router product that was running 8086 realmode *BSD, and suddenly so many stupid pointer bugs were immediately revealed )by a segfault) instead of the code just silently going off into the weeds. And when we started NULLing out pointers as things were freed we found so many more; the sources of mysterious problems that customers had been reporting for months were suddenly obvious. So my subconscious tells me that NULLing out freed pointers (and the memory they point to) is just "safer", and we're spending all this time removing that safety; kind of like going through all the cars in the world to remove their seatbelts because they make driving less convenient, and airbags offer a similar type of protection...)
[libvirt RFC PATCH 5/5] conf: replace virDomainDeviceInfoClear with virDomainDeviceInfoFreeContents
In every case where virDomainDeviceInfoClear() is still being called,
the object containing the DeviceInfo is freed immediately thereafter
(yes, I checked them all!). This means we don't need to clear any of
the data in the DeviceInfo, just free memory that it points to.
Signed-off-by: Laine Stump
---
src/conf/device_conf.c | 15 ++-
src/conf/device_conf.h | 2 +-
src/conf/domain_conf.c | 40
3 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 714ac50762..743d7f7468 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -83,22 +83,19 @@ virZPCIDeviceAddressParseXML(xmlNodePtr node,
}
void
-virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
+virDomainDeviceInfoFreeContents(virDomainDeviceInfoPtr info)
{
-VIR_FREE(info->alias);
-memset(>addr, 0, sizeof(info->addr));
-info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
-VIR_FREE(info->romfile);
-VIR_FREE(info->loadparm);
-info->isolationGroup = 0;
-info->isolationGroupLocked = false;
+g_free(info->alias);
+g_free(info->romfile);
+g_free(info->loadparm);
}
+
void
virDomainDeviceInfoFree(virDomainDeviceInfoPtr info)
{
if (info) {
-virDomainDeviceInfoClear(info);
+virDomainDeviceInfoFreeContents(info);
g_free(info);
}
}
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index a51bdf10ee..444d1b16c6 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -181,7 +181,7 @@ struct _virDomainDeviceInfo {
bool isolationGroupLocked;
};
-void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
+void virDomainDeviceInfoFreeContents(virDomainDeviceInfoPtr info);
void virDomainDeviceInfoFree(virDomainDeviceInfoPtr info);
bool virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f2b00129e1..b8db94974b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1880,7 +1880,7 @@ void virDomainInputDefFree(virDomainInputDefPtr def)
if (!def)
return;
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def->source.evdev);
g_free(def->virtio);
g_free(def);
@@ -2210,7 +2210,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def)
g_free(def->domain_name);
g_free(def->blkdeviotune.group_name);
g_free(def->virtio);
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
virObjectUnref(def->privateData);
g_free(def);
@@ -2342,7 +2342,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr
def)
if (!def)
return;
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def->virtio);
g_free(def);
@@ -2408,7 +2408,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def)
virObjectUnref(def->src);
g_free(def->dst);
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def->virtio);
virObjectUnref(def->privateData);
g_free(def->binary);
@@ -2471,7 +2471,7 @@ virDomainVsockDefFree(virDomainVsockDefPtr vsock)
return;
virObjectUnref(vsock->privateData);
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(vsock->virtio);
g_free(vsock);
}
@@ -2556,7 +2556,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
virNetDevIPInfoClear(>guestIP);
virNetDevIPInfoClear(>hostIP);
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def->filter);
virHashFree(def->filterparams);
@@ -2806,7 +2806,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
}
virObjectUnref(def->source);
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def);
}
@@ -2835,7 +2835,7 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr
def)
break;
}
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def);
}
@@ -2855,7 +2855,7 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
if (!def)
return;
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
for (i = 0; i < def->ncodecs; i++)
virDomainSoundCodecDefFree(def->codecs[i]);
@@ -2895,7 +2895,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr
def)
if (!def)
return;
-virDomainDeviceInfoClear(>info);
+virDomainDeviceInfoFreeContents(>info);
g_free(def->virtio);
g_free(def);
@@ -2906,7 +2906,7 @@ void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def)
if (!def)
retur
[libvirt RFC PATCH 4/5] conf: don't call virDomainDeviceInfoClear from virDomainDeviceInfoParseXML
The DeviceInfo object sent to virDomainDeviceInfoParseXML() was
allocated with g_new0() just prior to the call (although in each case
there is some other code in between, none of it touches the
DeviceInfo); pretty clearly, the call to virDomainDeviceInfoClear() at
the top of virDomainDeviceInfoParseXML() was just overly paranoid
"safety code", and is unnecessary.
Likewise, the call to clear the device info on return when there is a
parse failure is also unnecessary, since in every case the caller
immediately frees the object after the failure.
So let's remove these.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 4
1 file changed, 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8b8ef38f5..f2b00129e1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6533,8 +6533,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *rombar = NULL;
g_autofree char *aliasStr = NULL;
-virDomainDeviceInfoClear(info);
-
cur = node->children;
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
@@ -6611,8 +6609,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt,
ret = 0;
cleanup:
-if (ret < 0)
-virDomainDeviceInfoClear(info);
return ret;
}
--
2.29.2
[libvirt RFC PATCH 2/5] conf: rename virDomainHostdevSubsysSCSIClear
This function is only called from one place, and the object being
"cleared" is never again referenced before being freed, so it doesn't
need to be "cleared", just change its name to *FreeContents() so that
we can remove its VIR_FREEs with a "clear" conscience (pun intended).
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3b9d0da232..3505d29dbe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2997,13 +2997,13 @@ virDomainHostdevDefNew(void)
static void
-virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc)
+virDomainHostdevSubsysSCSIFreeContents(virDomainHostdevSubsysSCSIPtr scsisrc)
{
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
virObjectUnref(scsisrc->u.iscsi.src);
scsisrc->u.iscsi.src = NULL;
} else {
-VIR_FREE(scsisrc->u.host.adapter);
+g_free(scsisrc->u.host.adapter);
virObjectUnref(scsisrc->u.host.src);
scsisrc->u.host.src = NULL;
}
@@ -3048,7 +3048,7 @@ virDomainHostdevDefFreeContents(virDomainHostdevDefPtr
def)
case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS:
switch ((virDomainHostdevSubsysType) def->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
-virDomainHostdevSubsysSCSIClear(>source.subsys.u.scsi);
+virDomainHostdevSubsysSCSIFreeContents(>source.subsys.u.scsi);
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
g_free(def->source.subsys.u.scsi_host.wwpn);
--
2.29.2
[libvirt RFC PATCH 3/5] conf: replace pointless VIR_FREEs with g_free in virDomainVideoDefClear()
This function does a memset(0) at the end anyway, so there's no point in VIR_FREE vs g_free. Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3505d29dbe..d8b8ef38f5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2960,13 +2960,13 @@ virDomainVideoDefClear(virDomainVideoDefPtr def) virDomainDeviceInfoClear(>info); if (def->accel) -VIR_FREE(def->accel->rendernode); -VIR_FREE(def->accel); -VIR_FREE(def->res); -VIR_FREE(def->virtio); +g_free(def->accel->rendernode); +g_free(def->accel); +g_free(def->res); +g_free(def->virtio); if (def->driver) -VIR_FREE(def->driver->vhost_user_binary); -VIR_FREE(def->driver); +g_free(def->driver->vhost_user_binary); +g_free(def->driver); virObjectUnref(def->privateData); memset(def, 0, sizeof(*def)); -- 2.29.2
[libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions
I've looked at a few of these, and one thing I've found is that very often we have a function called somethingSomethingClear(), and: 1) The only places it is ever called will immediately free the memory of the object as soon as they clear it. and very possibly 2) It doesn't actually *clear* everything. Some items are cleared via VIR_FREE(), but then some of the other pointers call bobLoblawFree(def->bobloblaw) and then don't actually set def->bobloblaw to NULL - so the functions aren't actually "Clearing", they're "Freeing and then clearing a few things, but not everything". So I'm wondering if it is worthwhile to A) audit all the *Clear() functions and rename the functions that don't actually need to clear the contents to be, e.g. bobLobLawFreeContents(), while also replacing VIR_FREE with g_free(). (this is what I've done in these 5 patches) or if we should B) just do the wholesale approach and (as danpb suggested last week) change all VIR_FREE in *Clear() functions to g_free(), and put a "memset(obj, 0, sizeof(*obj))" at the end of each function, ignoring whether or not we actually need that. (B) would obviously be much faster to get done, and simpler for everyone to keep track of what's happening, but of course it is less efficient. Very likely this efficiency is completely meaningless in the large scheme (even in the medium or small scheme). (I actually like the idea of 0'ing out *everything*[*] when we're done with it, extra cycles be damned! I think of the two choices above, after going through this exercise, I'd say (B) is a more reasonable choice, but since I tried this, I thought I'd send it and see if anyone else has an opinion (or different idea) [*](including all those places I've replaced VIR_FREE with g_free in the name of "progress?") Laine Stump (5): conf: rename and narrow scope of virDomainHostdevDefClear() conf: rename virDomainHostdevSubsysSCSIClear conf: replace pointless VIR_FREEs with g_free in virDomainVideoDefClear() conf: don't call virDomainDeviceInfoClear from virDomainDeviceInfoParseXML conf: replace virDomainDeviceInfoClear with virDomainDeviceInfoFreeContents src/conf/device_conf.c | 15 +++- src/conf/device_conf.h | 2 +- src/conf/domain_conf.c | 81 src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 5 files changed, 47 insertions(+), 53 deletions(-) -- 2.29.2
[libvirt RFC PATCH 1/5] conf: rename and narrow scope of virDomainHostdevDefClear()
This function had the name "Clear", but some of its fields were being
cleared, and others were just being freed, but the pointers not
cleared. In fact, all the uses of virDomainHostdevDefClear() ended up
freeing the memory containing the HosdevDef immediately after clearing
it.
So it is acceptable to change the VIR_FREEs in this function to
g_frees, but that would be confusing to future me, who will have
forgotten that a "cleared" hostdevdef is never re-used. In order to
eliminate the confusion, we can rename the function to
virDomainHostdevDefFreeContents(), which is more accurate (doesn't
make false claims).
In the meantime, this function was only ever called from other
functions within the same file, so there is no need for it to be
declared in domain_conf.h, much less be listed among the exported
functions in libvirt_private.syms, so lets just make this renamed
function static for efficiency's sake.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 19 +++
src/conf/domain_conf.h | 1 -
src/libvirt_private.syms | 1 -
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 17e0d1ed31..3b9d0da232 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1339,6 +1339,7 @@ static virClassPtr virDomainXMLOptionClass;
static void virDomainObjDispose(void *obj);
static void virDomainXMLOptionDispose(void *obj);
+static void virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def);
static void
virDomainChrSourceDefFormat(virBufferPtr buf,
@@ -2430,7 +2431,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
g_free(def->data.direct.linkdev);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
-virDomainHostdevDefClear(>data.hostdev.def);
+virDomainHostdevDefFree(>data.hostdev.def);
break;
default:
break;
@@ -2531,7 +2532,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
-virDomainHostdevDefClear(>data.hostdev.def);
+virDomainHostdevDefFree(>data.hostdev.def);
break;
case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -3009,7 +3010,8 @@
virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc)
}
-void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
+static void
+virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def)
{
if (!def)
return;
@@ -3030,13 +3032,13 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr
def)
case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
switch ((virDomainHostdevCapsType) def->source.caps.type) {
case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE:
-VIR_FREE(def->source.caps.u.storage.block);
+g_free(def->source.caps.u.storage.block);
break;
case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC:
-VIR_FREE(def->source.caps.u.misc.chardev);
+g_free(def->source.caps.u.misc.chardev);
break;
case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET:
-VIR_FREE(def->source.caps.u.net.ifname);
+g_free(def->source.caps.u.net.ifname);
virNetDevIPInfoClear(>source.caps.u.net.ip);
break;
case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST:
@@ -3049,7 +3051,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
virDomainHostdevSubsysSCSIClear(>source.subsys.u.scsi);
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
-VIR_FREE(def->source.subsys.u.scsi_host.wwpn);
+g_free(def->source.subsys.u.scsi_host.wwpn);
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
@@ -3061,6 +3063,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
}
}
+
void virDomainTPMDefFree(virDomainTPMDefPtr def)
{
if (!def)
@@ -3089,7 +3092,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
return;
/* free all subordinate objects */
-virDomainHostdevDefClear(def);
+virDomainHostdevDefFreeContents(def);
/* If there is a parentnet device object, it will handle freeing
* the memory.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e263e6098b..415826134d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3126,7 +3126,6 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVideoDef, virDomainVideoDefFree);
void virDomainVideoDefClear(virDomainVideoDefPtr def);
virDomainHostdevDefPtr virDomainHostdevDefNew(void);
-void virDomainHostdevDefClear(virDomainHostdevDefPtr def);
void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainHubDefFree(virDomainHubDefPtr def);
void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def);
diff --git a/src/libvirt_private.syms b/sr
[libvirt PATCH 8/8] vmware: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump
---
src/vmware/vmware_conf.c | 4 ++--
src/vmware/vmware_driver.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index 55cd1d6f2d..f905962b85 100644
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -56,8 +56,8 @@ vmwareFreeDriver(struct vmware_driver *driver)
virObjectUnref(driver->domains);
virObjectUnref(driver->caps);
virObjectUnref(driver->xmlopt);
-VIR_FREE(driver->vmrun);
-VIR_FREE(driver);
+g_free(driver->vmrun);
+g_free(driver);
}
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 481c59ef3c..5f1edeb7f5 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -110,8 +110,8 @@ vmwareDataFreeFunc(void *data)
{
vmwareDomainPtr dom = data;
-VIR_FREE(dom->vmxPath);
-VIR_FREE(dom);
+g_free(dom->vmxPath);
+g_free(dom);
}
static int
--
2.29.2
[libvirt PATCH 6/8] qemu: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump
---
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_driver.c | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 14d05cfe3b..13efdcbad1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -95,7 +95,7 @@ qemuJobFreePrivate(void *opaque)
return;
qemuMigrationParamsFree(priv->migParams);
-VIR_FREE(priv);
+g_free(priv);
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 53ac3b0c17..58236f1ae2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17677,12 +17677,12 @@ qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr
resdata)
{
size_t i = 0;
-VIR_FREE(resdata->name);
-VIR_FREE(resdata->vcpus);
+g_free(resdata->name);
+g_free(resdata->vcpus);
for (i = 0; i < resdata->nstats; i++)
virResctrlMonitorStatsFree(resdata->stats[i]);
-VIR_FREE(resdata->stats);
-VIR_FREE(resdata);
+g_free(resdata->stats);
+g_free(resdata);
}
--
2.29.2
[libvirt PATCH 2/8] conf: convert VIR_FREE to g_free in other functions that free their arg
Previous patches have converted VIR_FREE to g_free in functions with
names ending in Free() and Dispose(), but there are a few similar
functions with names that don't fit that pattern, but server the same
purpose (and thus can survive the same conversion). in particular
*Free*(), and *Unref().
Signed-off-by: Laine Stump
---
src/conf/capabilities.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index dd3321db9a..69d9bb0e38 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -118,10 +118,10 @@ virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr
cell)
virCapabilitiesClearHostNUMACellCPUTopology(cell->cpus, cell->ncpus);
-VIR_FREE(cell->cpus);
-VIR_FREE(cell->siblings);
-VIR_FREE(cell->pageinfo);
-VIR_FREE(cell);
+g_free(cell->cpus);
+g_free(cell->siblings);
+g_free(cell->pageinfo);
+g_free(cell);
}
static void
@@ -129,9 +129,9 @@ virCapabilitiesFreeGuestMachine(virCapsGuestMachinePtr
machine)
{
if (machine == NULL)
return;
-VIR_FREE(machine->name);
-VIR_FREE(machine->canonical);
-VIR_FREE(machine);
+g_free(machine->name);
+g_free(machine->canonical);
+g_free(machine);
}
static void
@@ -141,13 +141,13 @@ virCapabilitiesFreeGuestDomain(virCapsGuestDomainPtr dom)
if (dom == NULL)
return;
-VIR_FREE(dom->info.emulator);
-VIR_FREE(dom->info.loader);
+g_free(dom->info.emulator);
+g_free(dom->info.loader);
for (i = 0; i < dom->info.nmachines; i++)
virCapabilitiesFreeGuestMachine(dom->info.machines[i]);
-VIR_FREE(dom->info.machines);
+g_free(dom->info.machines);
-VIR_FREE(dom);
+g_free(dom);
}
void
@@ -157,17 +157,17 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
if (guest == NULL)
return;
-VIR_FREE(guest->arch.defaultInfo.emulator);
-VIR_FREE(guest->arch.defaultInfo.loader);
+g_free(guest->arch.defaultInfo.emulator);
+g_free(guest->arch.defaultInfo.loader);
for (i = 0; i < guest->arch.defaultInfo.nmachines; i++)
virCapabilitiesFreeGuestMachine(guest->arch.defaultInfo.machines[i]);
-VIR_FREE(guest->arch.defaultInfo.machines);
+g_free(guest->arch.defaultInfo.machines);
for (i = 0; i < guest->arch.ndomains; i++)
virCapabilitiesFreeGuestDomain(guest->arch.domains[i]);
-VIR_FREE(guest->arch.domains);
+g_free(guest->arch.domains);
-VIR_FREE(guest);
+g_free(guest);
}
@@ -177,7 +177,7 @@ virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool)
if (!pool)
return;
-VIR_FREE(pool);
+g_free(pool);
}
@@ -190,7 +190,7 @@ virCapabilitiesHostNUMAUnref(virCapsHostNUMAPtr caps)
if (g_atomic_int_dec_and_test(>refs)) {
g_ptr_array_unref(caps->cells);
-VIR_FREE(caps);
+g_free(caps);
}
}
@@ -409,7 +409,7 @@ virCapabilitiesFreeMachines(virCapsGuestMachinePtr
*machines,
virCapabilitiesFreeGuestMachine(machines[i]);
machines[i] = NULL;
}
-VIR_FREE(machines);
+g_free(machines);
}
/**
--
2.29.2
[libvirt PATCH 5/8] remote: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/remote/remote_daemon_stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c index b4a3cb6741..57816eb10e 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -419,7 +419,7 @@ int daemonFreeClientStream(virNetServerClientPtr client, } virObjectUnref(stream->st); -VIR_FREE(stream); +g_free(stream); return ret; } -- 2.29.2
[libvirt PATCH 7/8] util: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump
---
src/util/virconf.c | 4 ++--
src/util/virerror.c | 4 ++--
src/util/virobject.c | 2 +-
src/util/virstring.c | 4 ++--
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index d85fc32b64..16107bce96 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -151,11 +151,11 @@ virConfFreeValue(virConfValuePtr val)
return;
if (val->type == VIR_CONF_STRING &&
val->str != NULL)
-VIR_FREE(val->str);
+g_free(val->str);
if (val->type == VIR_CONF_LIST &&
val->list != NULL)
virConfFreeList(val->list);
-VIR_FREE(val);
+g_free(val);
}
virConfPtr
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 14054add41..708081414a 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -158,7 +158,7 @@ virLastErrFreeData(void *data)
if (!err)
return;
virResetError(err);
-VIR_FREE(err);
+g_free(err);
}
@@ -477,7 +477,7 @@ void
virFreeError(virErrorPtr err)
{
virResetError(err);
-VIR_FREE(err);
+g_free(err);
}
/**
diff --git a/src/util/virobject.c b/src/util/virobject.c
index ce40ffae22..370a64bd6d 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -655,5 +655,5 @@ virObjectListFreeCount(void *list,
for (i = 0; i < count; i++)
virObjectUnref(((void **)list)[i]);
-VIR_FREE(list);
+g_free(list);
}
diff --git a/src/util/virstring.c b/src/util/virstring.c
index c98435388a..c3e64007fe 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -108,9 +108,9 @@ virStringListFreeCount(char **strings,
return;
for (i = 0; i < count; i++)
-VIR_FREE(strings[i]);
+g_free(strings[i]);
-VIR_FREE(strings);
+g_free(strings);
}
--
2.29.2
[libvirt PATCH 1/8] esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE
Invocations of the macro ESX_VI__TEMPLATE__FREE() will free the main
object (referenced as "item") that's pointing to all the things being
VIR_FREEd in the body, so it is safe for all the pointers in item to
just be g_freed rather that VIR_FREEd.
Signed-off-by: Laine Stump
---
src/esx/esx_vi.c | 18 +-
src/esx/esx_vi_types.c | 24
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 7e05318fdc..2eb8048858 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -805,19 +805,19 @@ ESX_VI__TEMPLATE__FREE(Context,
virMutexDestroy(item->sessionLock);
esxVI_CURL_Free(>curl);
-VIR_FREE(item->url);
-VIR_FREE(item->ipAddress);
-VIR_FREE(item->username);
-VIR_FREE(item->password);
+g_free(item->url);
+g_free(item->ipAddress);
+g_free(item->username);
+g_free(item->password);
esxVI_ServiceContent_Free(>service);
esxVI_UserSession_Free(>session);
-VIR_FREE(item->sessionLock);
+g_free(item->sessionLock);
esxVI_Datacenter_Free(>datacenter);
-VIR_FREE(item->datacenterPath);
+g_free(item->datacenterPath);
esxVI_ComputeResource_Free(>computeResource);
-VIR_FREE(item->computeResourcePath);
+g_free(item->computeResourcePath);
esxVI_HostSystem_Free(>hostSystem);
-VIR_FREE(item->hostSystemName);
+g_free(item->hostSystemName);
esxVI_SelectionSpec_Free(>selectSet_folderToChildEntity);
esxVI_SelectionSpec_Free(>selectSet_hostSystemToParent);
esxVI_SelectionSpec_Free(>selectSet_hostSystemToVm);
@@ -1419,7 +1419,7 @@ ESX_VI__TEMPLATE__ALLOC(Response)
/* esxVI_Response_Free */
ESX_VI__TEMPLATE__FREE(Response,
{
-VIR_FREE(item->content);
+g_free(item->content);
xmlFreeDoc(item->document);
})
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index 6821587e44..4d3617e0a8 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -900,8 +900,8 @@ ESX_VI__TEMPLATE__ALLOC(AnyType)
ESX_VI__TEMPLATE__FREE(AnyType,
{
xmlFreeNode(item->node);
-VIR_FREE(item->other);
-VIR_FREE(item->value);
+g_free(item->other);
+g_free(item->value);
})
const char *
@@ -1117,7 +1117,7 @@ ESX_VI__TEMPLATE__FREE(String,
{
esxVI_String_Free(>_next);
-VIR_FREE(item->value);
+g_free(item->value);
})
/* esxVI_String_Validate */
@@ -1421,7 +1421,7 @@ ESX_VI__TEMPLATE__ALLOC(DateTime)
/* esxVI_DateTime_Free */
ESX_VI__TEMPLATE__FREE(DateTime,
{
-VIR_FREE(item->value);
+g_free(item->value);
})
/* esxVI_DateTime_Validate */
@@ -1564,8 +1564,8 @@ ESX_VI__TEMPLATE__ALLOC(Fault);
/* esxVI_Fault_Free */
ESX_VI__TEMPLATE__FREE(Fault,
{
-VIR_FREE(item->faultcode);
-VIR_FREE(item->faultstring);
+g_free(item->faultcode);
+g_free(item->faultstring);
})
/* esxVI_Fault_Validate */
@@ -1595,7 +1595,7 @@ ESX_VI__TEMPLATE__ALLOC(MethodFault);
/* esxVI_MethodFault_Free */
ESX_VI__TEMPLATE__FREE(MethodFault,
{
-VIR_FREE(item->_actualType);
+g_free(item->_actualType);
})
int
@@ -1638,8 +1638,8 @@ ESX_VI__TEMPLATE__FREE(ManagedObjectReference,
{
esxVI_ManagedObjectReference_Free(>_next);
-VIR_FREE(item->type);
-VIR_FREE(item->value);
+g_free(item->type);
+g_free(item->value);
})
/* esxVI_ManagedObjectReference_DeepCopy */
@@ -1732,17 +1732,17 @@ ESX_VI__TEMPLATE__ALLOC(Event)
ESX_VI__TEMPLATE__FREE(Event,
{
esxVI_Event_Free(>_next);
-VIR_FREE(item->_actualType);
+g_free(item->_actualType);
esxVI_Int_Free(>key);
esxVI_Int_Free(>chainId);
esxVI_DateTime_Free(>createdTime);
-VIR_FREE(item->userName);
+g_free(item->userName);
/* FIXME: datacenter is currently ignored */
/* FIXME: computeResource is currently ignored */
/* FIXME: host is currently ignored */
esxVI_VmEventArgument_Free(>vm);
-VIR_FREE(item->fullFormattedMessage);
+g_free(item->fullFormattedMessage);
})
/* esxVI_Event_Validate */
--
2.29.2
[libvirt PATCH 0/8] More VIR_FREE removals
Only 90 this time. These are all functions that behave similar to the *Free() functions, but their names don't end in "Free" so I missed them last time. Laine Stump (8): esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE conf: convert VIR_FREE to g_free in other functions that free their arg locking: convert VIR_FREE to g_free in other functions that free their arg openvz: convert VIR_FREE to g_free in other functions that free their arg remote: convert VIR_FREE to g_free in other functions that free their arg qemu: convert VIR_FREE to g_free in other functions that free their arg util: convert VIR_FREE to g_free in other functions that free their arg vmware: convert VIR_FREE to g_free in other functions that free their arg src/conf/capabilities.c | 38 +++ src/esx/esx_vi.c | 18 +++ src/esx/esx_vi_types.c| 24 +-- src/locking/lock_manager.c| 4 ++-- src/openvz/openvz_conf.c | 2 +- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 8 +++ src/remote/remote_daemon_stream.c | 2 +- src/util/virconf.c| 4 ++-- src/util/virerror.c | 4 ++-- src/util/virobject.c | 2 +- src/util/virstring.c | 4 ++-- src/vmware/vmware_conf.c | 4 ++-- src/vmware/vmware_driver.c| 4 ++-- 14 files changed, 60 insertions(+), 60 deletions(-) -- 2.29.2
[libvirt PATCH 3/8] locking: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/locking/lock_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index db724eb30f..90ec5cdfbe 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -245,8 +245,8 @@ void virLockManagerPluginUnref(virLockManagerPluginPtr plugin) return; } -VIR_FREE(plugin->name); -VIR_FREE(plugin); +g_free(plugin->name); +g_free(plugin); } #else /* !WITH_DLFCN_H */ void virLockManagerPluginUnref(virLockManagerPluginPtr plugin G_GNUC_UNUSED) -- 2.29.2
[libvirt PATCH 4/8] openvz: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/openvz/openvz_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 1783dce233..041a031a3a 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -478,7 +478,7 @@ openvzFreeDriver(struct openvz_driver *driver) virObjectUnref(driver->xmlopt); virObjectUnref(driver->domains); virObjectUnref(driver->caps); -VIR_FREE(driver); +g_free(driver); } -- 2.29.2
[PATCH 6/7] qemu: plug config from into qemu commandline
Signed-off-by: Laine Stump
---
src/qemu/qemu_command.c | 15 ---
.../net-virtio-teaming-hostdev.args | 40 +++
tests/qemuxml2argvtest.c | 3 ++
3 files changed, 53 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09516d407f..30663c933e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4427,6 +4427,7 @@ qemuBuildPCIHostdevDevStr(const virDomainDef *def,
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
int backend = pcisrc->backend;
+virDomainNetTeamingInfoPtr teaming;
/* caller has to assign proper passthrough backend type */
switch ((virDomainHostdevSubsysPCIBackendType)backend) {
@@ -4461,11 +4462,15 @@ qemuBuildPCIHostdevDevStr(const virDomainDef *def,
if (qemuBuildRomStr(, dev->info) < 0)
return NULL;
-if (dev->parentnet && dev->parentnet->teaming &&
-dev->parentnet->teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT
&&
-dev->parentnet->teaming->persistent) {
-virBufferAsprintf(, ",failover_pair_id=%s",
- dev->parentnet->teaming->persistent);
+if (dev->parentnet)
+teaming = dev->parentnet->teaming;
+else
+teaming = dev->teaming;
+
+if (teaming &&
+teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT &&
+teaming->persistent) {
+virBufferAsprintf(, ",failover_pair_id=%s", teaming->persistent);
}
return virBufferContentAndReset();
diff --git a/tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args
b/tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args
new file mode 100644
index 00..19e7260843
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args
@@ -0,0 +1,40 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i386 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
+-netdev user,id=hostua-backup0 \
+-device virtio-net-pci,failover=on,netdev=hostua-backup0,id=ua-backup0,\
+mac=00:11:22:33:44:55,bus=pci.0,addr=0x3 \
+-netdev user,id=hostua-backup1 \
+-device virtio-net-pci,failover=on,netdev=hostua-backup1,id=ua-backup1,\
+mac=66:44:33:22:11:00,bus=pci.0,addr=0x4 \
+-device vfio-pci,host=:03:07.1,id=hostdev0,bus=pci.0,addr=0x5,\
+failover_pair_id=ua-backup0 \
+-device vfio-pci,host=:03:07.2,id=hostdev1,bus=pci.0,addr=0x6,\
+failover_pair_id=ua-backup1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index faa71a7a16..93d6a608bf 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1503,6 +1503,9 @@ mymain(void)
QEMU_CAPS_VIRTIO_NET_FAILOVER,
QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST_PARSE_ERROR("net-virtio-teaming", NONE);
+DO_TEST("net-virtio-teaming-hostdev",
+QEMU_CAPS_VIRTIO_NET_FAILOVER,
+QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST("net-eth", NONE);
DO_TEST("net-eth-ifname", NONE);
DO_TEST("net-eth-names", NONE);
--
2.29.2
[PATCH 3/7] conf: separate Parse/Format functions for virDomainNetTeamingInfo
In preparation for using the same element in two places, split the
parsing/formating for that subelement out of the virDomainNetDef
functions into their own functions.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 74 +-
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bbf54c90ba..3fe8517f39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10629,6 +10629,34 @@
virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def,
}
+static int
+virDomainNetTeamingInfoParseXML(xmlXPathContextPtr ctxt,
+virDomainNetTeamingInfoPtr *teaming)
+{
+g_autofree char *typeStr = virXPathString("string(./teaming/@type)", ctxt);
+g_autofree char *persistentStr =
virXPathString("string(./teaming/@persistent)", ctxt);
+g_autoptr(virDomainNetTeamingInfo) tmpTeaming = NULL;
+int tmpType;
+
+if (!typeStr && !persistentStr)
+return 0;
+
+tmpTeaming = g_new0(virDomainNetTeamingInfo, 1);
+
+if ((tmpType = virDomainNetTeamingTypeFromString(typeStr)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown teaming type '%s'"),
+ typeStr);
+return -1;
+}
+
+tmpTeaming->type = tmpType;
+tmpTeaming->persistent = g_steal_pointer();
+*teaming = g_steal_pointer();
+return 0;
+}
+
+
static virDomainNetDefPtr
virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr node,
@@ -10683,8 +10711,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *vhostuser_type = NULL;
g_autofree char *trustGuestRxFilters = NULL;
g_autofree char *vhost_path = NULL;
-g_autofree char *teamingType = NULL;
-g_autofree char *teamingPersistent = NULL;
const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL;
if (!(def = virDomainNetDefNew(xmlopt)))
@@ -10895,10 +10921,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (!vhost_path && (tmp = virXMLPropString(cur, "vhost")))
vhost_path = virFileSanitizePath(tmp);
VIR_FREE(tmp);
-} else if (virXMLNodeNameEqual(cur, "teaming") &&
- !teamingType && !teamingPersistent) {
-teamingType = virXMLPropString(cur, "type");
-teamingPersistent = virXMLPropString(cur, "persistent");
}
}
cur = cur->next;
@@ -11447,23 +11469,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
-if (teamingType || teamingPersistent) {
-def->teaming = g_new0(virDomainNetTeamingInfo, 1);
-
-if (teamingType) {
-int tmpTeaming;
-
-if ((tmpTeaming = virDomainNetTeamingTypeFromString(teamingType))
<= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown teaming type '%s'"),
- teamingType);
-goto error;
-}
-def->teaming->type = tmpTeaming;
-}
-
-def->teaming->persistent = g_steal_pointer();
-}
+if (virDomainNetTeamingInfoParseXML(ctxt, >teaming) < 0)
+goto error;
rv = virXPathULong("string(./tune/sndbuf)", ctxt, >tune.sndbuf);
if (rv >= 0) {
@@ -25513,6 +25520,19 @@ virDomainChrSourceReconnectDefFormat(virBufferPtr buf,
}
+static void
+virDomainNetTeamingInfoFormat(virDomainNetTeamingInfoPtr teaming,
+ virBufferPtr buf)
+{
+if (teaming && teaming->type != VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
+virBufferAsprintf(buf, "type));
+virBufferEscapeString(buf, " persistent='%s'", teaming->persistent);
+virBufferAddLit(buf, "/>\n");
+}
+}
+
+
int
virDomainNetDefFormat(virBufferPtr buf,
virDomainNetDefPtr def,
@@ -25830,12 +25850,8 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "\n");
}
-if (def->teaming && def->teaming->type !=
VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
-virBufferAsprintf(buf, "teaming->type));
-virBufferEscapeString(buf, " persistent='%s'",
def->teaming->persistent);
-virBufferAddLit(buf, "/>\n");
-}
+virDomainNetTeamingInfoFormat(def->teaming, buf);
+
if (def->linkstate) {
virBufferAsprintf(buf, "\n",
virDomainNetInterfaceLinkStateTypeToString(def->linkstate));
--
2.29.2
[PATCH 4/7] schema: separate teaming element definition from interface element
Signed-off-by: Laine Stump --- docs/schemas/domaincommon.rng | 39 --- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7a2706a4fb..31960fb7cf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3529,23 +3529,7 @@ - - - - -persistent - - - - -transient - - - - - - - + @@ -3581,6 +3565,27 @@ + + + + + + +persistent + + + + +transient + + + + + + + + + -- 2.29.2
[PATCH 7/7] news: document support for in
Signed-off-by: Laine Stump --- NEWS.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index c6ae6a6c60..a9a1a61119 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -35,6 +35,12 @@ v7.1.0 (unreleased) ``virNetworkGetXMLDesc()``, and ``virDomainScreenshot()``, APIs have been implemented in the Hyper-V driver. + * Support element in plain devices + +This is useful when libvirt doesn't have the privileges necessary +to set the hostdev device's MAC address (which is a necessary +part of the alternate ). + * **Improvements** * **Bug fixes** -- 2.29.2
[PATCH 1/7] conf: make teaming info an official type
This struct was previously defined only within virDomainNetDef where
it was used, but I need to also use it in virDomainHostdevDef, so move
the internal struct out to its own "official" struct and give it the
standard typedef duo and *Free() function.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 11 +++
src/conf/domain_conf.h | 12
src/conf/virconftypes.h | 3 +++
src/libvirt_private.syms | 1 +
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f2207bdf6..7d7acb940a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2476,6 +2476,17 @@ virDomainVsockDefFree(virDomainVsockDefPtr vsock)
}
+void
+virDomainNetTeamingInfoFree(virDomainNetTeamingInfoPtr teaming)
+{
+if (!teaming)
+return;
+
+g_free(teaming->persistent);
+g_free(teaming);
+}
+
+
void
virDomainNetDefFree(virDomainNetDefPtr def)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8b1c8643be..92fe588b3f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -956,6 +956,11 @@ typedef enum {
VIR_DOMAIN_NET_TEAMING_TYPE_LAST
} virDomainNetTeamingType;
+struct _virDomainNetTeamingInfo {
+virDomainNetTeamingType type;
+char *persistent; /* alias name of persistent device */
+};
+
/* link interface states */
typedef enum {
VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT = 0, /* Default link state
(up) */
@@ -1033,10 +1038,7 @@ struct _virDomainNetDef {
char *tap;
char *vhost;
} backend;
-struct {
-virDomainNetTeamingType type;
-char *persistent; /* alias name of persistent device */
-} teaming;
+virDomainNetTeamingInfo teaming;
union {
virDomainChrSourceDefPtr vhostuser;
struct {
@@ -3100,6 +3102,8 @@ void virDomainActualNetDefFree(virDomainActualNetDefPtr
def);
virDomainVsockDefPtr virDomainVsockDefNew(virDomainXMLOptionPtr xmlopt);
void virDomainVsockDefFree(virDomainVsockDefPtr vsock);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVsockDef, virDomainVsockDefFree);
+void virDomainNetTeamingInfoFree(virDomainNetTeamingInfoPtr teaming);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetTeamingInfo,
virDomainNetTeamingInfoFree);
void virDomainNetDefFree(virDomainNetDefPtr def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetDef, virDomainNetDefFree);
void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def);
diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
index 9042a2b34f..c51241cfa5 100644
--- a/src/conf/virconftypes.h
+++ b/src/conf/virconftypes.h
@@ -246,6 +246,9 @@ typedef virDomainNVRAMDef *virDomainNVRAMDefPtr;
typedef struct _virDomainNetDef virDomainNetDef;
typedef virDomainNetDef *virDomainNetDefPtr;
+typedef struct _virDomainNetTeamingInfo virDomainNetTeamingInfo;
+typedef virDomainNetTeamingInfo *virDomainNetTeamingInfoPtr;
+
typedef struct _virDomainOSDef virDomainOSDef;
typedef virDomainOSDef *virDomainOSDefPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 837986845f..affa2df323 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -537,6 +537,7 @@ virDomainNetRemove;
virDomainNetRemoveHostdev;
virDomainNetResolveActualType;
virDomainNetSetModelString;
+virDomainNetTeamingInfoFree;
virDomainNetTypeFromString;
virDomainNetTypeSharesHostView;
virDomainNetTypeToString;
--
2.29.2
[PATCH 0/7] support (aka "QEMU virtio failover") with plain
This is explained in excruciating detail in Patch 5, but in short, allowing the element to be in a plain will permit someone who is running libvirt unprivileged, or in a container with no access to a PF device, to use the feature, which encapsulates a lot of functionality related to assigning an SRIOV network device to a guest as a part of a failover bond device (the other device in the pair is an emulated virtio NIC), which in turn permits the guest to be migrated by transparently unplugging the SRIOV NIC prior to migration, and plugging a new one in after migration is completed. (Previously we required for this feature, but that type of device needs to send netlink messages to the PF of the SRIOV VF that's being assigned, and that simply isn't possible sometimes.) Laine Stump (7): conf: make teaming info an official type conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo conf: separate Parse/Format functions for virDomainNetTeamingInfo schema: separate teaming element definition from interface element conf: parse/format element in plain qemu: plug config from into qemu commandline news: document support for in NEWS.rst | 6 ++ docs/formatdomain.rst | 51 +++ docs/schemas/domaincommon.rng | 42 + src/conf/domain_conf.c| 87 +-- src/conf/domain_conf.h| 13 ++- src/conf/domain_validate.c| 45 +++--- src/conf/virconftypes.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 17 ++-- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_migration.c | 4 +- src/qemu/qemu_validate.c | 42 - .../net-virtio-teaming-hostdev.args | 40 + .../net-virtio-teaming-hostdev.xml| 48 ++ tests/qemuxml2argvtest.c | 3 + .../net-virtio-teaming-hostdev.xml| 64 ++ tests/qemuxml2xmltest.c | 3 + 17 files changed, 384 insertions(+), 87 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.xml create mode 100644 tests/qemuxml2xmloutdata/net-virtio-teaming-hostdev.xml -- 2.29.2
[PATCH 5/7] conf: parse/format element in plain
The element in allows pairing two interfaces
together as a simple "failover bond" network device in a guest. One of
the devices will be the "transient" interface - it will be preferred
for all network traffic when it is present, but may be removed when
necessary, in particular during migration, when traffic will instead
go through the other interface of the pair - the "persistent"
interface. As it happens, in the QEMU implementation of this teaming
pair (called "virtio failover" in QEMU) the transient interface is
always a host network device assigned to the guest using VFIO (aka
"hostdev"); the persistent interface is always an emulated virtio NIC.
When support was initially added for , it was written to
require that the transient/hostdev device be defined using ; this was done because the virtio failover
implementation in QEMU and the virtio guest driver demands that the
two interfaces in the pair have matching MAC addresses, and the only
way libvirt can guarantee the MAC address of a hostdev network device
is to use , whose main purpose is to
configure the device's MAC address. (note that in turn requires that the network device be an SRIOV
VF (Virtual Function), as that is the only type of network device
whose MAC address we can set in a way that will survive the device's
driver init in the guest).
It has recently come up that some users are unable to use
because they are running in a container environment where libvirt
doesn't have the necessary privileges or resources to set the VF's MAC
address (because setting the VF MAC is done via the same device's PF
(Physical Function), and the PF is not exposed to libvirt's container.
At the same time, they *are* able to set the VF's MAC address in
advance of staring up libvirt in the container. So they could
theoretically use the feature if libvirt just skipped the
"setting the MAC address" part.
Fortunately, that is *exactly* the difference between (a "hostdev VF") and (a "plain hostdev" - it
could be *any PCI device; libvirt doesn't know what type of PCI device
it is, and doesn't care).
But what *is* still needed is for libvirt to provide a small bit of
information on the commandline argument for the hostdev, telling QEMU
that this device will be part of a team ("failover pair"), and the id
of the other device in the pair.
So, what we need to do is add support for the element to
plain , and that is what this patch does.
(actually, this patch adds parsing/formatting of the element
in . The next patch will actually wire that into the qemu
driver.)
Signed-off-by: Laine Stump
---
docs/formatdomain.rst | 51 +++
docs/schemas/domaincommon.rng | 3 +
src/conf/domain_conf.c| 5 ++
src/conf/domain_conf.h| 1 +
src/conf/domain_validate.c| 19 ++
.../net-virtio-teaming-hostdev.xml| 48 ++
.../net-virtio-teaming-hostdev.xml| 64 +++
tests/qemuxml2xmltest.c | 3 +
8 files changed, 194 insertions(+)
create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.xml
create mode 100644 tests/qemuxml2xmloutdata/net-virtio-teaming-hostdev.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 2493be595f..eafd6b3396 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -4837,6 +4837,22 @@ support in the hypervisor and the guest network driver).
...
+The second interface in this example is referencing a network that is
+a pool of SRIOV VFs (i.e. a "hostdev network"). You could instead
+directly reference an SRIOV VF device:
+
+::
+
+ ...
+
+
+
+
+
+
+
+ ...
+
The element required attribute ``type`` will be set to either
``"persistent"`` to indicate a device that should always be present in the
domain, or ``"transient"`` to indicate a device that may periodically be
@@ -4858,6 +4874,41 @@ once migration is completed; while migration is taking
place, network traffic
will use the virtio NIC. (Of course the emulated virtio NIC and the hostdev NIC
must be connected to the same subnet for bonding to work properly).
+:since:`Since 7.1.0` The element can also be added to a
+plain device.
+
+::
+
+ ...
+
+
+
+
+
+
+
+ ...
+
+This device must be a network device, but not necessarily an SRIOV
+VF. Using plain rather than or is useful if the
+device that will be assigned with VFIO is a standard NIC (not a VF) or
+if libvirt doesn't have the necessary resources and privileges to set
+the VF's MAC address (e.g. if libvirt is running unprivileged, or in a
+container). This of course means that the user (or another
+application) is responsible for setting the MAC address
[PATCH 2/7] conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo
To make it easier to split out the parsing/formatting of the
element into separate functions (so we can more easily add the
element to , change its virDomainNetDef so that it
points to a virDomainNetTeamingInfo rather than containing one.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 31
src/conf/domain_conf.h | 2 +-
src/conf/domain_validate.c | 26 ---
src/qemu/qemu_command.c| 10 -
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_migration.c | 4 ++--
src/qemu/qemu_validate.c | 42 --
7 files changed, 63 insertions(+), 54 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7d7acb940a..bbf54c90ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2542,7 +2542,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
g_free(def->backend.tap);
g_free(def->backend.vhost);
-g_free(def->teaming.persistent);
+virDomainNetTeamingInfoFree(def->teaming);
g_free(def->virtPortProfile);
g_free(def->script);
g_free(def->downscript);
@@ -11447,18 +11447,23 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
-if (teamingType) {
-int tmpTeaming;
+if (teamingType || teamingPersistent) {
+def->teaming = g_new0(virDomainNetTeamingInfo, 1);
-if ((tmpTeaming = virDomainNetTeamingTypeFromString(teamingType)) <=
0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown teaming type '%s'"),
- teamingType);
-goto error;
+if (teamingType) {
+int tmpTeaming;
+
+if ((tmpTeaming = virDomainNetTeamingTypeFromString(teamingType))
<= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown teaming type '%s'"),
+ teamingType);
+goto error;
+}
+def->teaming->type = tmpTeaming;
}
-def->teaming.type = tmpTeaming;
+
+def->teaming->persistent = g_steal_pointer();
}
-def->teaming.persistent = g_steal_pointer();
rv = virXPathULong("string(./tune/sndbuf)", ctxt, >tune.sndbuf);
if (rv >= 0) {
@@ -25825,10 +25830,10 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "\n");
}
-if (def->teaming.type != VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
+if (def->teaming && def->teaming->type !=
VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
virBufferAsprintf(buf, "teaming.type));
-virBufferEscapeString(buf, " persistent='%s'",
def->teaming.persistent);
+ virDomainNetTeamingTypeToString(def->teaming->type));
+virBufferEscapeString(buf, " persistent='%s'",
def->teaming->persistent);
virBufferAddLit(buf, "/>\n");
}
if (def->linkstate) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 92fe588b3f..fb695a212b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1038,7 +1038,7 @@ struct _virDomainNetDef {
char *tap;
char *vhost;
} backend;
-virDomainNetTeamingInfo teaming;
+virDomainNetTeamingInfoPtr teaming;
union {
virDomainChrSourceDefPtr vhostuser;
struct {
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index b6f53886cd..703946b3e5 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1507,18 +1507,20 @@ virDomainNetDefValidate(const virDomainNetDef *net)
return -1;
}
-if (net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT) {
-if (!net->teaming.persistent) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("teaming persistent attribute must be set if
teaming type is 'transient'"));
-return -1;
-}
-} else {
-if (net->teaming.persistent) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("teaming persistent attribute not allowed if
teaming type is '%s'"),
- virDomainNetTeamingTypeToString(net->teaming.type));
-return -1;
+if (net->teaming) {
+if (net->teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT) {
+if (!net->teaming->persistent) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("teaming persistent attribute must be set if
teaming type is 'transient'"));
+return -1;
+}
+} else {
+if (net->teaming->persistent) {
+virRepo
[PATCH] qemu: match alias when looking for proper to detach.
Previously we only checked MAC address and PCI address (or CCW
address). This is not enough information in cases where PCI address
isn't provided and multiple interfaces have the same MAC address (for
example, a virtio + hostdev "teaming" pair - their MAC addresses are
always the same).
Resolves: https://bugzilla.redhat.com/1926190
Signed-off-by: Laine Stump
---
Arguably, it would be nice to overhaul the device matching used for
virDomainDeviceDetach for *all* the device types, as they could all be
matched by looking at alias (and PCI address, for that matter). On the
other hand, for all other device types there are already enough fields
being matched to assure a unique match even without looking at
alias/PCI address, and this patch is intended to fix a current problem
being experienced in the wild, meaning it's likely that it will need
to be backported to stable branches, and I'd rather not force
backporting a sweeping change to stable branches just to bring in a
fix for one corner case)
src/conf/domain_conf.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 07e6f39256..8f2207bdf6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16431,6 +16431,11 @@ virDomainNetFindIdx(virDomainDefPtr def,
virDomainNetDefPtr net)
>info.addr.ccw))
continue;
+if (net->info.alias &&
+STRNEQ_NULLABLE(def->nets[i]->info.alias, net->info.alias)) {
+continue;
+}
+
if (matchidx >= 0) {
/* there were multiple matches on mac address, and no
* qualifying guest-side PCI/CCW address was given, so we must
--
2.29.2
[libvirt PATCH 11/15] security: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump --- src/security/security_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_manager.c b/src/security/security_manager.c index be81ee5e44..67ede11981 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -55,7 +55,7 @@ void virSecurityManagerDispose(void *obj) if (mgr->drv->close) mgr->drv->close(mgr); -VIR_FREE(mgr->privateData); +g_free(mgr->privateData); } -- 2.29.2
[libvirt PATCH 09/15] logging: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump --- src/logging/log_handler.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 45a4763525..a77c1e0250 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -352,7 +352,7 @@ virLogHandlerDispose(void *obj) handler->inhibitor(false, handler->opaque); virLogHandlerLogFileFree(handler->files[i]); } -VIR_FREE(handler->files); +g_free(handler->files); } -- 2.29.2
[libvirt PATCH 08/15] hypervisor: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump --- src/hypervisor/virhostdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index 09995a52ed..743aaa84d6 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -135,7 +135,7 @@ virHostdevManagerDispose(void *obj) virObjectUnref(hostdevMgr->activeSCSIVHostHostdevs); virObjectUnref(hostdevMgr->activeMediatedHostdevs); virObjectUnref(hostdevMgr->activeNVMeHostdevs); -VIR_FREE(hostdevMgr->stateDir); +g_free(hostdevMgr->stateDir); } static virHostdevManagerPtr -- 2.29.2
[libvirt PATCH 15/15] datatypes: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
src/datatypes.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 1db38c5aa6..e36f3a2d24 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -347,7 +347,7 @@ virDomainDispose(void *obj)
virUUIDFormat(domain->uuid, uuidstr);
VIR_DEBUG("release domain %p %s %s", domain, domain->name, uuidstr);
-VIR_FREE(domain->name);
+g_free(domain->name);
virObjectUnref(domain->conn);
}
@@ -409,7 +409,7 @@ virNetworkDispose(void *obj)
virUUIDFormat(network->uuid, uuidstr);
VIR_DEBUG("release network %p %s %s", network, network->name, uuidstr);
-VIR_FREE(network->name);
+g_free(network->name);
virObjectUnref(network->conn);
}
@@ -528,8 +528,8 @@ virInterfaceDispose(void *obj)
virInterfacePtr iface = obj;
VIR_DEBUG("release interface %p %s", iface, iface->name);
-VIR_FREE(iface->name);
-VIR_FREE(iface->mac);
+g_free(iface->name);
+g_free(iface->mac);
virObjectUnref(iface->conn);
}
@@ -603,7 +603,7 @@ virStoragePoolDispose(void *obj)
if (pool->privateDataFreeFunc)
pool->privateDataFreeFunc(pool->privateData);
-VIR_FREE(pool->name);
+g_free(pool->name);
virObjectUnref(pool->conn);
}
@@ -676,9 +676,9 @@ virStorageVolDispose(void *obj)
if (vol->privateDataFreeFunc)
vol->privateDataFreeFunc(vol->privateData);
-VIR_FREE(vol->key);
-VIR_FREE(vol->name);
-VIR_FREE(vol->pool);
+g_free(vol->key);
+g_free(vol->name);
+g_free(vol->pool);
virObjectUnref(vol->conn);
}
@@ -734,8 +734,8 @@ virNodeDeviceDispose(void *obj)
virNodeDevicePtr dev = obj;
VIR_DEBUG("release dev %p %s", dev, dev->name);
-VIR_FREE(dev->name);
-VIR_FREE(dev->parentName);
+g_free(dev->name);
+g_free(dev->parentName);
virObjectUnref(dev->conn);
}
@@ -798,7 +798,7 @@ virSecretDispose(void *obj)
virUUIDFormat(secret->uuid, uuidstr);
VIR_DEBUG("release secret %p %s", secret, uuidstr);
-VIR_FREE(secret->usageID);
+g_free(secret->usageID);
virObjectUnref(secret->conn);
}
@@ -910,7 +910,7 @@ virNWFilterDispose(void *obj)
virUUIDFormat(nwfilter->uuid, uuidstr);
VIR_DEBUG("release nwfilter %p %s %s", nwfilter, nwfilter->name, uuidstr);
-VIR_FREE(nwfilter->name);
+g_free(nwfilter->name);
virObjectUnref(nwfilter->conn);
}
@@ -972,8 +972,8 @@ virNWFilterBindingDispose(void *obj)
VIR_DEBUG("release binding %p %s", binding, binding->portdev);
-VIR_FREE(binding->portdev);
-VIR_FREE(binding->filtername);
+g_free(binding->portdev);
+g_free(binding->filtername);
virObjectUnref(binding->conn);
}
@@ -1030,7 +1030,7 @@ virDomainCheckpointDispose(void *obj)
virDomainCheckpointPtr checkpoint = obj;
VIR_DEBUG("release checkpoint %p %s", checkpoint, checkpoint->name);
-VIR_FREE(checkpoint->name);
+g_free(checkpoint->name);
virObjectUnref(checkpoint->domain);
}
@@ -1086,7 +1086,7 @@ virDomainSnapshotDispose(void *obj)
virDomainSnapshotPtr snapshot = obj;
VIR_DEBUG("release snapshot %p %s", snapshot, snapshot->name);
-VIR_FREE(snapshot->name);
+g_free(snapshot->name);
virObjectUnref(snapshot->domain);
}
@@ -1222,7 +1222,7 @@ virAdmServerDispose(void *obj)
virAdmServerPtr srv = obj;
VIR_DEBUG("release server srv=%p name=%s", srv, srv->name);
-VIR_FREE(srv->name);
+g_free(srv->name);
virObjectUnref(srv->conn);
}
--
2.29.2
[libvirt PATCH 10/15] rpc: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
src/rpc/virnetclient.c| 4 ++--
src/rpc/virnetdaemon.c| 6 +++---
src/rpc/virnetlibsshsession.c | 18 +-
src/rpc/virnetsaslcontext.c | 2 +-
src/rpc/virnetserver.c| 8
src/rpc/virnetserverservice.c | 2 +-
src/rpc/virnetsocket.c| 6 +++---
src/rpc/virnetsshsession.c| 8
src/rpc/virnettlscontext.c| 6 +++---
9 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 2eabacd6e8..edd65941ac 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -747,12 +747,12 @@ void virNetClientDispose(void *obj)
for (i = 0; i < client->nprograms; i++)
virObjectUnref(client->programs[i]);
-VIR_FREE(client->programs);
+g_free(client->programs);
g_main_loop_unref(client->eventLoop);
g_main_context_unref(client->eventCtx);
-VIR_FREE(client->hostname);
+g_free(client->hostname);
if (client->sock)
virNetSocketRemoveIOCallback(client->sock);
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index e337ff1fde..2c18da790b 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -99,9 +99,9 @@ virNetDaemonDispose(void *obj)
for (i = 0; i < dmn->nsignals; i++) {
sigaction(dmn->signals[i]->signum, >signals[i]->oldaction, NULL);
-VIR_FREE(dmn->signals[i]);
+g_free(dmn->signals[i]);
}
-VIR_FREE(dmn->signals);
+g_free(dmn->signals);
VIR_FORCE_CLOSE(dmn->sigread);
VIR_FORCE_CLOSE(dmn->sigwrite);
if (dmn->sigwatch > 0)
@@ -109,7 +109,7 @@ virNetDaemonDispose(void *obj)
#endif /* !WIN32 */
VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd);
-VIR_FREE(dmn->stateStopThread);
+g_free(dmn->stateStopThread);
virHashFree(dmn->servers);
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index 48ef914c70..8814487557 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -132,18 +132,18 @@ virNetLibsshSessionDispose(void *obj)
for (i = 0; i < sess->nauths; i++) {
virSecureEraseString(sess->auths[i]->password);
-VIR_FREE(sess->auths[i]->password);
-VIR_FREE(sess->auths[i]->filename);
-VIR_FREE(sess->auths[i]);
+g_free(sess->auths[i]->password);
+g_free(sess->auths[i]->filename);
+g_free(sess->auths[i]);
}
-VIR_FREE(sess->auths);
+g_free(sess->auths);
-VIR_FREE(sess->channelCommand);
-VIR_FREE(sess->hostname);
-VIR_FREE(sess->knownHostsFile);
-VIR_FREE(sess->authPath);
-VIR_FREE(sess->username);
+g_free(sess->channelCommand);
+g_free(sess->hostname);
+g_free(sess->knownHostsFile);
+g_free(sess->authPath);
+g_free(sess->username);
}
static virClassPtr virNetLibsshSessionClass;
diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c
index 9253771787..8a6250120d 100644
--- a/src/rpc/virnetsaslcontext.c
+++ b/src/rpc/virnetsaslcontext.c
@@ -699,7 +699,7 @@ void virNetSASLSessionDispose(void *obj)
if (sasl->conn)
sasl_dispose(>conn);
-VIR_FREE(sasl->callbacks);
+g_free(sasl->callbacks);
}
#ifdef __APPLE__
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index fa63acbb09..f0b866a962 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -909,21 +909,21 @@ void virNetServerDispose(void *obj)
virNetServerPtr srv = obj;
size_t i;
-VIR_FREE(srv->name);
+g_free(srv->name);
virThreadPoolFree(srv->workers);
for (i = 0; i < srv->nservices; i++)
virObjectUnref(srv->services[i]);
-VIR_FREE(srv->services);
+g_free(srv->services);
for (i = 0; i < srv->nprograms; i++)
virObjectUnref(srv->programs[i]);
-VIR_FREE(srv->programs);
+g_free(srv->programs);
for (i = 0; i < srv->nclients; i++)
virObjectUnref(srv->clients[i]);
-VIR_FREE(srv->clients);
+g_free(srv->clients);
}
void virNetServerClose(virNetServerPtr srv)
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 317cacf25b..73232e3747 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -419,7 +419,7 @@ void virNetServerServiceDispose(void *obj)
for (i = 0; i < svc->nsocks; i++)
virObjectUnref(svc->socks[i]);
-VIR_FREE(svc->socks);
+g_free(svc->socks);
virObjectUnref(svc->tls);
}
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index e56d43ba85..d0dc59c10c 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1356,9 +1356,9 @@ void virNetSocketDispose(void *obj)
virProce
[libvirt PATCH 14/15] tests: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
tests/virfilecachetest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/virfilecachetest.c b/tests/virfilecachetest.c
index 34e0d0ab2f..4d65c0c6ce 100644
--- a/tests/virfilecachetest.c
+++ b/tests/virfilecachetest.c
@@ -43,7 +43,7 @@ static void
testFileCacheObjDispose(void *opaque)
{
testFileCacheObjPtr obj = opaque;
-VIR_FREE(obj->data);
+g_free(obj->data);
}
--
2.29.2
[libvirt PATCH 12/15] util: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
src/util/virdnsmasq.c | 2 +-
src/util/virfilecache.c | 4 ++--
src/util/virmdev.c | 2 +-
src/util/virnvme.c | 2 +-
src/util/virpci.c | 2 +-
src/util/virresctrl.c | 40
src/util/virscsi.c | 2 +-
src/util/virscsivhost.c | 2 +-
src/util/virusb.c | 2 +-
9 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 653d46bef9..3eee4a5a84 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -604,7 +604,7 @@ dnsmasqCapsDispose(void *obj)
dnsmasqCapsPtr caps = obj;
virBitmapFree(caps->flags);
-VIR_FREE(caps->binaryPath);
+g_free(caps->binaryPath);
}
static int dnsmasqCapsOnceInit(void)
diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index a761a01b25..14b2885505 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -74,8 +74,8 @@ virFileCacheDispose(void *obj)
{
virFileCachePtr cache = obj;
-VIR_FREE(cache->dir);
-VIR_FREE(cache->suffix);
+g_free(cache->dir);
+g_free(cache->suffix);
virHashFree(cache->table);
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 1f5c2cba30..db40fe8cc3 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -297,7 +297,7 @@ virMediatedDeviceListDispose(void *obj)
}
list->count = 0;
-VIR_FREE(list->devs);
+g_free(list->devs);
}
diff --git a/src/util/virnvme.c b/src/util/virnvme.c
index af297552c1..a1fc9aab57 100644
--- a/src/util/virnvme.c
+++ b/src/util/virnvme.c
@@ -166,7 +166,7 @@ virNVMeDeviceListDispose(void *obj)
for (i = 0; i < list->count; i++)
virNVMeDeviceFree(list->devs[i]);
-VIR_FREE(list->devs);
+g_free(list->devs);
}
diff --git a/src/util/virpci.c b/src/util/virpci.c
index b8e8422aa0..8147ce11e9 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1667,7 +1667,7 @@ virPCIDeviceListDispose(void *obj)
}
list->count = 0;
-VIR_FREE(list->devs);
+g_free(list->devs);
}
int
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 4ee95aa232..29df51c652 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -210,18 +210,18 @@ virResctrlInfoDispose(void *obj)
if (level->types) {
for (j = 0; j < VIR_CACHE_TYPE_LAST; j++)
-VIR_FREE(level->types[j]);
+g_free(level->types[j]);
}
-VIR_FREE(level->types);
-VIR_FREE(level);
+g_free(level->types);
+g_free(level);
}
if (resctrl->monitor_info)
g_strfreev(resctrl->monitor_info->features);
-VIR_FREE(resctrl->membw_info);
-VIR_FREE(resctrl->levels);
-VIR_FREE(resctrl->monitor_info);
+g_free(resctrl->membw_info);
+g_free(resctrl->levels);
+g_free(resctrl->monitor_info);
}
@@ -396,30 +396,30 @@ virResctrlAllocDispose(void *obj)
continue;
for (k = 0; k < type->nsizes; k++)
-VIR_FREE(type->sizes[k]);
+g_free(type->sizes[k]);
for (k = 0; k < type->nmasks; k++)
virBitmapFree(type->masks[k]);
-VIR_FREE(type->sizes);
-VIR_FREE(type->masks);
-VIR_FREE(type);
+g_free(type->sizes);
+g_free(type->masks);
+g_free(type);
}
-VIR_FREE(level->types);
-VIR_FREE(level);
+g_free(level->types);
+g_free(level);
}
if (alloc->mem_bw) {
virResctrlAllocMemBWPtr mem_bw = alloc->mem_bw;
for (i = 0; i < mem_bw->nbandwidths; i++)
-VIR_FREE(mem_bw->bandwidths[i]);
-VIR_FREE(alloc->mem_bw->bandwidths);
-VIR_FREE(alloc->mem_bw);
+g_free(mem_bw->bandwidths[i]);
+g_free(alloc->mem_bw->bandwidths);
+g_free(alloc->mem_bw);
}
-VIR_FREE(alloc->id);
-VIR_FREE(alloc->path);
-VIR_FREE(alloc->levels);
+g_free(alloc->id);
+g_free(alloc->path);
+g_free(alloc->levels);
}
@@ -429,8 +429,8 @@ virResctrlMonitorDispose(void *obj)
virResctrlMonitorPtr monitor = obj;
virObjectUnref(monitor->alloc);
-VIR_FREE(monitor->id);
-VIR_FREE(monitor->path);
+g_free(monitor->id);
+g_free(monitor->path);
}
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 0c753c075e..2a9f6da76a 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -347,7 +347,7 @@ virSCSIDeviceListDispose(void *obj)
for (i = 0; i < list->count; i++)
virSCSIDeviceFree(list->devs[i]);
-VIR_FREE(list->devs);
+g_free(list->devs);
}
int
diff --git a/src/util/virscs
[libvirt PATCH 13/15] conf: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
src/conf/capabilities.c| 22 +++---
src/conf/checkpoint_conf.c | 2 +-
src/conf/domain_capabilities.c | 12
src/conf/domain_conf.c | 2 +-
src/conf/domain_event.c| 52 +-
src/conf/moment_conf.c | 6 ++--
src/conf/object_event.c| 4 +--
src/conf/snapshot_conf.c | 4 +--
src/conf/virsecretobj.c| 6 ++--
src/conf/virstorageobj.c | 4 +--
10 files changed, 57 insertions(+), 57 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 02fbf60b02..dd3321db9a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -232,39 +232,39 @@ virCapsDispose(void *object)
for (i = 0; i < caps->npools; i++)
virCapabilitiesFreeStoragePool(caps->pools[i]);
-VIR_FREE(caps->pools);
+g_free(caps->pools);
for (i = 0; i < caps->nguests; i++)
virCapabilitiesFreeGuest(caps->guests[i]);
-VIR_FREE(caps->guests);
+g_free(caps->guests);
for (i = 0; i < caps->host.nfeatures; i++)
-VIR_FREE(caps->host.features[i]);
-VIR_FREE(caps->host.features);
+g_free(caps->host.features[i]);
+g_free(caps->host.features);
if (caps->host.numa)
virCapabilitiesHostNUMAUnref(caps->host.numa);
for (i = 0; i < caps->host.nmigrateTrans; i++)
-VIR_FREE(caps->host.migrateTrans[i]);
-VIR_FREE(caps->host.migrateTrans);
+g_free(caps->host.migrateTrans[i]);
+g_free(caps->host.migrateTrans);
for (i = 0; i < caps->host.nsecModels; i++)
virCapabilitiesClearSecModel(>host.secModels[i]);
-VIR_FREE(caps->host.secModels);
+g_free(caps->host.secModels);
for (i = 0; i < caps->host.cache.nbanks; i++)
virCapsHostCacheBankFree(caps->host.cache.banks[i]);
virResctrlInfoMonFree(caps->host.cache.monitor);
-VIR_FREE(caps->host.cache.banks);
+g_free(caps->host.cache.banks);
for (i = 0; i < caps->host.memBW.nnodes; i++)
virCapsHostMemBWNodeFree(caps->host.memBW.nodes[i]);
virResctrlInfoMonFree(caps->host.memBW.monitor);
-VIR_FREE(caps->host.memBW.nodes);
+g_free(caps->host.memBW.nodes);
-VIR_FREE(caps->host.netprefix);
-VIR_FREE(caps->host.pagesSize);
+g_free(caps->host.netprefix);
+g_free(caps->host.pagesSize);
virCPUDefFree(caps->host.cpu);
virObjectUnref(caps->host.resctrl);
}
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index 16fb138a3e..f8704852e0 100644
--- a/src/conf/checkpoint_conf.c
+++ b/src/conf/checkpoint_conf.c
@@ -87,7 +87,7 @@ virDomainCheckpointDefDispose(void *obj)
for (i = 0; i < def->ndisks; i++)
virDomainCheckpointDiskDefClear(>disks[i]);
-VIR_FREE(def->disks);
+g_free(def->disks);
}
static int
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 407cf0348a..8605216cf7 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -84,16 +84,16 @@ virDomainCapsDispose(void *obj)
virDomainCapsStringValuesPtr values;
size_t i;
-VIR_FREE(caps->path);
-VIR_FREE(caps->machine);
+g_free(caps->path);
+g_free(caps->machine);
virObjectUnref(caps->cpu.custom);
virCPUDefFree(caps->cpu.hostModel);
virSEVCapabilitiesFree(caps->sev);
values = >os.loader.values;
for (i = 0; i < values->nvalues; i++)
-VIR_FREE(values->values[i]);
-VIR_FREE(values->values);
+g_free(values->values[i]);
+g_free(values->values);
}
@@ -104,11 +104,11 @@ virDomainCapsCPUModelsDispose(void *obj)
size_t i;
for (i = 0; i < cpuModels->nmodels; i++) {
-VIR_FREE(cpuModels->models[i].name);
+g_free(cpuModels->models[i].name);
g_strfreev(cpuModels->models[i].blockers);
}
-VIR_FREE(cpuModels->models);
+g_free(cpuModels->models);
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3f71297283..07e6f39256 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2699,7 +2699,7 @@ virDomainChrSourceDefDispose(void *obj)
if (def->seclabels) {
for (i = 0; i < def->nseclabels; i++)
virSecurityDeviceLabelDefFree(def->seclabels[i]);
-VIR_FREE(def->seclabels);
+g_free(def->seclabels);
}
}
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 0fde3481ed..726a792dae 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -387,9 +387,9 @@ virDomainEventIOErrorDispose(void *obj)
virDomainEventIOErrorPtr event = obj;
VIR_DEBUG("obj=%p", event);
-VIR_FREE(event->srcPath);
-VIR_F
[libvirt PATCH 06/15] interface: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump --- src/interface/interface_backend_netcf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index df04484c59..e40a4cb108 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -84,7 +84,7 @@ virNetcfDriverStateDispose(void *obj) if (_driver->lockFD != -1) virPidFileRelease(_driver->stateDir, "driver", _driver->lockFD); -VIR_FREE(_driver->stateDir); +g_free(_driver->stateDir); } -- 2.29.2
[libvirt PATCH 04/15] libxl: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump --- src/libxl/libxl_conf.c | 20 ++-- src/libxl/libxl_migration.c | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index de0fd66842..37a2c2e3cd 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -76,16 +76,16 @@ libxlDriverConfigDispose(void *obj) if (cfg->logger) libxlLoggerFree(cfg->logger); -VIR_FREE(cfg->configBaseDir); -VIR_FREE(cfg->configDir); -VIR_FREE(cfg->autostartDir); -VIR_FREE(cfg->logDir); -VIR_FREE(cfg->stateDir); -VIR_FREE(cfg->libDir); -VIR_FREE(cfg->saveDir); -VIR_FREE(cfg->autoDumpDir); -VIR_FREE(cfg->lockManagerName); -VIR_FREE(cfg->channelDir); +g_free(cfg->configBaseDir); +g_free(cfg->configDir); +g_free(cfg->autostartDir); +g_free(cfg->logDir); +g_free(cfg->stateDir); +g_free(cfg->libDir); +g_free(cfg->saveDir); +g_free(cfg->autoDumpDir); +g_free(cfg->lockManagerName); +g_free(cfg->channelDir); virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); } diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 72b6cb32cb..c56fdd98ab 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -228,7 +228,7 @@ libxlMigrationDstArgsDispose(void *obj) libxlMigrationDstArgs *args = obj; libxlMigrationCookieFree(args->migcookie); -VIR_FREE(args->socks); +g_free(args->socks); virObjectUnref(args->conn); virObjectUnref(args->vm); } -- 2.29.2
[libvirt PATCH 07/15] access: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump --- src/access/viraccessmanager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c index 02e4464ef5..c81d0840b3 100644 --- a/src/access/viraccessmanager.c +++ b/src/access/viraccessmanager.c @@ -190,7 +190,7 @@ static void virAccessManagerDispose(void *object) if (mgr->drv->cleanup) mgr->drv->cleanup(mgr); -VIR_FREE(mgr->privateData); +g_free(mgr->privateData); } -- 2.29.2
[libvirt PATCH 05/15] qemu: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
src/qemu/qemu_agent.c| 2 +-
src/qemu/qemu_capabilities.c | 10 +--
src/qemu/qemu_conf.c | 122 +--
src/qemu/qemu_domain.c | 10 +--
src/qemu/qemu_monitor.c | 4 +-
5 files changed, 74 insertions(+), 74 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index af0397e6e2..51cc00c618 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -177,7 +177,7 @@ static void qemuAgentDispose(void *obj)
if (agent->cb && agent->cb->destroy)
(agent->cb->destroy)(agent, agent->vm);
virCondDestroy(>notify);
-VIR_FREE(agent->buffer);
+g_free(agent->buffer);
g_main_context_unref(agent->context);
virResetError(>lastError);
}
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c57864f602..d41b4a4753 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1982,12 +1982,12 @@ void virQEMUCapsDispose(void *obj)
virBitmapFree(qemuCaps->flags);
-VIR_FREE(qemuCaps->package);
-VIR_FREE(qemuCaps->kernelVersion);
-VIR_FREE(qemuCaps->binary);
-VIR_FREE(qemuCaps->hostCPUSignature);
+g_free(qemuCaps->package);
+g_free(qemuCaps->kernelVersion);
+g_free(qemuCaps->binary);
+g_free(qemuCaps->hostCPUSignature);
-VIR_FREE(qemuCaps->gicCapabilities);
+g_free(qemuCaps->gicCapabilities);
virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index eae94bb5a2..35d0b6515c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -306,78 +306,78 @@ static void virQEMUDriverConfigDispose(void *obj)
virBitmapFree(cfg->namespaces);
g_strfreev(cfg->cgroupDeviceACL);
-VIR_FREE(cfg->uri);
-
-VIR_FREE(cfg->configBaseDir);
-VIR_FREE(cfg->configDir);
-VIR_FREE(cfg->autostartDir);
-VIR_FREE(cfg->logDir);
-VIR_FREE(cfg->swtpmLogDir);
-VIR_FREE(cfg->stateDir);
-VIR_FREE(cfg->swtpmStateDir);
-VIR_FREE(cfg->slirpStateDir);
-VIR_FREE(cfg->dbusStateDir);
-
-VIR_FREE(cfg->libDir);
-VIR_FREE(cfg->cacheDir);
-VIR_FREE(cfg->saveDir);
-VIR_FREE(cfg->snapshotDir);
-VIR_FREE(cfg->checkpointDir);
-VIR_FREE(cfg->channelTargetDir);
-VIR_FREE(cfg->nvramDir);
-
-VIR_FREE(cfg->defaultTLSx509certdir);
-VIR_FREE(cfg->defaultTLSx509secretUUID);
-
-VIR_FREE(cfg->vncTLSx509certdir);
-VIR_FREE(cfg->vncTLSx509secretUUID);
-VIR_FREE(cfg->vncListen);
-VIR_FREE(cfg->vncPassword);
-VIR_FREE(cfg->vncSASLdir);
-
-VIR_FREE(cfg->spiceTLSx509certdir);
-VIR_FREE(cfg->spiceListen);
-VIR_FREE(cfg->spicePassword);
-VIR_FREE(cfg->spiceSASLdir);
-
-VIR_FREE(cfg->chardevTLSx509certdir);
-VIR_FREE(cfg->chardevTLSx509secretUUID);
-
-VIR_FREE(cfg->vxhsTLSx509certdir);
-VIR_FREE(cfg->vxhsTLSx509secretUUID);
-
-VIR_FREE(cfg->nbdTLSx509certdir);
-VIR_FREE(cfg->nbdTLSx509secretUUID);
-
-VIR_FREE(cfg->migrateTLSx509certdir);
-VIR_FREE(cfg->migrateTLSx509secretUUID);
-
-VIR_FREE(cfg->backupTLSx509certdir);
-VIR_FREE(cfg->backupTLSx509secretUUID);
+g_free(cfg->uri);
+
+g_free(cfg->configBaseDir);
+g_free(cfg->configDir);
+g_free(cfg->autostartDir);
+g_free(cfg->logDir);
+g_free(cfg->swtpmLogDir);
+g_free(cfg->stateDir);
+g_free(cfg->swtpmStateDir);
+g_free(cfg->slirpStateDir);
+g_free(cfg->dbusStateDir);
+
+g_free(cfg->libDir);
+g_free(cfg->cacheDir);
+g_free(cfg->saveDir);
+g_free(cfg->snapshotDir);
+g_free(cfg->checkpointDir);
+g_free(cfg->channelTargetDir);
+g_free(cfg->nvramDir);
+
+g_free(cfg->defaultTLSx509certdir);
+g_free(cfg->defaultTLSx509secretUUID);
+
+g_free(cfg->vncTLSx509certdir);
+g_free(cfg->vncTLSx509secretUUID);
+g_free(cfg->vncListen);
+g_free(cfg->vncPassword);
+g_free(cfg->vncSASLdir);
+
+g_free(cfg->spiceTLSx509certdir);
+g_free(cfg->spiceListen);
+g_free(cfg->spicePassword);
+g_free(cfg->spiceSASLdir);
+
+g_free(cfg->chardevTLSx509certdir);
+g_free(cfg->chardevTLSx509secretUUID);
+
+g_free(cfg->vxhsTLSx509certdir);
+g_free(cfg->vxhsTLSx509secretUUID);
+
+g_free(cfg->nbdTLSx509certdir);
+g_free(cfg->nbdTLSx509secretUUID);
+
+g_free(cfg->migrateTLSx509certdir);
+g_free(cfg->migrateTLSx509secretUUID);
+
+g_free(cfg->backupTLSx509certdir);
+g_free(cfg->backupTLSx509secretUUID);
while (cfg->nhugetlbfs) {
cfg->nhugetlbfs--;
-VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
+g_free(cfg->hugetl
[libvirt PATCH 03/15] bhyve: replace VIR_FREE with g_free in all *Dispose() functions
Signed-off-by: Laine Stump
---
src/bhyve/bhyve_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c
index 9012eb8b5f..0f60aac9a0 100644
--- a/src/bhyve/bhyve_conf.c
+++ b/src/bhyve/bhyve_conf.c
@@ -97,7 +97,7 @@ virBhyveDriverConfigDispose(void *obj)
{
virBhyveDriverConfigPtr cfg = obj;
-VIR_FREE(cfg->firmwareDir);
+g_free(cfg->firmwareDir);
}
void
--
2.29.2
[libvirt PATCH 02/15] rpc: eliminate static function virNetLibsshSessionAuthMethodsFree()
This function is only called from one place, and has, well... not a
*misleading* name, but it doesn't fit the standard frame of functions
that end in "Free" (it doesn't actually free the object pointed to by
its argument, but frees *some parts* of the content of the object).
Rather than try to think up an appropriate name, let's just move the
meat of this function into its one and only caller,
virNetLibsshSessionDispose(), which will allow us to convert its
VIR_FREEs into g_free in a future patch.
Signed-off-by: Laine Stump
---
src/rpc/virnetlibsshsession.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index 76934c7c0b..48ef914c70 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -108,26 +108,12 @@ struct _virNetLibsshSession {
size_t bufStart;
};
-static void
-virNetLibsshSessionAuthMethodsFree(virNetLibsshSessionPtr sess)
-{
-size_t i;
-
-for (i = 0; i < sess->nauths; i++) {
-virSecureEraseString(sess->auths[i]->password);
-g_free(sess->auths[i]->password);
-VIR_FREE(sess->auths[i]->filename);
-VIR_FREE(sess->auths[i]);
-}
-
-VIR_FREE(sess->auths);
-sess->nauths = 0;
-}
-
static void
virNetLibsshSessionDispose(void *obj)
{
virNetLibsshSessionPtr sess = obj;
+size_t i;
+
VIR_DEBUG("sess=0x%p", sess);
if (!sess)
@@ -144,7 +130,14 @@ virNetLibsshSessionDispose(void *obj)
ssh_free(sess->session);
}
-virNetLibsshSessionAuthMethodsFree(sess);
+for (i = 0; i < sess->nauths; i++) {
+virSecureEraseString(sess->auths[i]->password);
+VIR_FREE(sess->auths[i]->password);
+VIR_FREE(sess->auths[i]->filename);
+VIR_FREE(sess->auths[i]);
+}
+
+VIR_FREE(sess->auths);
VIR_FREE(sess->channelCommand);
VIR_FREE(sess->hostname);
--
2.29.2
[libvirt PATCH 01/15] conf: simplify virDomainCapsDispose()
virDomainCapsDispose() was the only caller of
virDomainCapsStringValuesFree(), which 1) didn't actually free the
object it was called with, but only cleared it, making it less
mechanical to convert from VIR_FREE to g_free (since it's not
immediately obvious from looking at virDomainCapsStringValuesFree()
that the pointers being cleared will never again be used).
We could have renamed the function to virDomainCapsStringValuesClear()
to side-step the confusion of what the function actually does, but
that would just make the upcoming switch from VIR_FREE to g_free
require more thought. But since there is only a single caller to the
function, and it is a vir*Dispose() function (indicating that the
object containing the virDomainCapsStringValues is going to be freed
immediately after the function finishes), and thus VIR_FREE() *could*
be safely replaced by g_free()), we instead just move the contents of
virDomainCapsStringValuesFree() into virDomainCapsDispose() (and
*that* function will be trivially converted in an upcoming
"mechanical" patch).
Signed-off-by: Laine Stump
---
src/conf/domain_capabilities.c | 21 ++---
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 837c571b45..407cf0348a 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -65,20 +65,6 @@ static int virDomainCapsOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virDomainCaps);
-static void
-virDomainCapsStringValuesFree(virDomainCapsStringValuesPtr values)
-{
-size_t i;
-
-if (!values || !values->values)
-return;
-
-for (i = 0; i < values->nvalues; i++)
-VIR_FREE(values->values[i]);
-VIR_FREE(values->values);
-}
-
-
void
virSEVCapabilitiesFree(virSEVCapability *cap)
{
@@ -95,6 +81,8 @@ static void
virDomainCapsDispose(void *obj)
{
virDomainCapsPtr caps = obj;
+virDomainCapsStringValuesPtr values;
+size_t i;
VIR_FREE(caps->path);
VIR_FREE(caps->machine);
@@ -102,7 +90,10 @@ virDomainCapsDispose(void *obj)
virCPUDefFree(caps->cpu.hostModel);
virSEVCapabilitiesFree(caps->sev);
-virDomainCapsStringValuesFree(>os.loader.values);
+values = >os.loader.values;
+for (i = 0; i < values->nvalues; i++)
+VIR_FREE(values->values[i]);
+VIR_FREE(values->values);
}
--
2.29.2
[libvirt PATCH 00/15] eliminate VIR_FREE in all *Dispose() functions
A *Dispose() function is similar to a *Free() function, except that 1) the object is always sent as a void* so it has to be typecast into some other object-specific pointer at the top of the function, and 2) it frees all the resources inside the object, but never frees the object itself (this is done by the caller, somewhere deep in the bowels of virObjectDispose or something I guess; frankly I've always ignored the details simply "because I could"). The important point is that the contents of the object are never referenced in any way after return from the Dispose function, so it is unnecessary to clear any pointers, ergo (always wanted to use that word!) it's completely safe to replace all VIR_FREEs in a *Dispose() function with g_free (as long as there's nothing within the Dispose function itself that depends on the pointers being cleared). After this series is applied, there will be exactly 0 instances of VIR_FREE in any *Dispose() (or *Free()) function. As with the *Free() series, almost all were accomplished by directly replacing VIR_FREE with g_free, but there were a couple oddities that needed separate patches just to call them out: * Patch 1 & 2 - in both cases a Dispose() function was the only caller to a *Free() function that didn't fit the normal pattern of a *Free() function. Since each of the Free()s had only one caller (their respective *Dispose() parents), their contents were just moved into the caller, clearing the way for their VIR_FREEs to be g_free-ified (in later patches, along with the other *Dispose() functions in the same directories). 220 VIR_FREE uses eliminated in this series, so a total of 762 for the two series combined (nearly 20% of all remaining VIR_FREEs). Laine Stump (15): conf: simplify virDomainCapsDispose() rpc: eliminate static function virNetLibsshSessionAuthMethodsFree() bhyve: replace VIR_FREE with g_free in all *Dispose() functions libxl: replace VIR_FREE with g_free in all *Dispose() functions qemu: replace VIR_FREE with g_free in all *Dispose() functions interface: replace VIR_FREE with g_free in all *Dispose() functions access: replace VIR_FREE with g_free in all *Dispose() functions hypervisor: replace VIR_FREE with g_free in all *Dispose() functions logging: replace VIR_FREE with g_free in all *Dispose() functions rpc: replace VIR_FREE with g_free in all *Dispose() functions security: replace VIR_FREE with g_free in all *Dispose() functions util: replace VIR_FREE with g_free in all *Dispose() functions conf: replace VIR_FREE with g_free in all *Dispose() functions tests: replace VIR_FREE with g_free in all *Dispose() functions datatypes: replace VIR_FREE with g_free in all *Dispose() functions src/access/viraccessmanager.c | 2 +- src/bhyve/bhyve_conf.c | 2 +- src/conf/capabilities.c | 22 ++--- src/conf/checkpoint_conf.c | 2 +- src/conf/domain_capabilities.c | 29 ++ src/conf/domain_conf.c | 2 +- src/conf/domain_event.c | 52 +- src/conf/moment_conf.c | 6 +- src/conf/object_event.c | 4 +- src/conf/snapshot_conf.c| 4 +- src/conf/virsecretobj.c | 6 +- src/conf/virstorageobj.c| 4 +- src/datatypes.c | 34 +++ src/hypervisor/virhostdev.c | 2 +- src/interface/interface_backend_netcf.c | 2 +- src/libxl/libxl_conf.c | 20 ++-- src/libxl/libxl_migration.c | 2 +- src/logging/log_handler.c | 2 +- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_capabilities.c| 10 +- src/qemu/qemu_conf.c| 122 src/qemu/qemu_domain.c | 10 +- src/qemu/qemu_monitor.c | 4 +- src/rpc/virnetclient.c | 4 +- src/rpc/virnetdaemon.c | 6 +- src/rpc/virnetlibsshsession.c | 37 +++ src/rpc/virnetsaslcontext.c | 2 +- src/rpc/virnetserver.c | 8 +- src/rpc/virnetserverservice.c | 2 +- src/rpc/virnetsocket.c | 6 +- src/rpc/virnetsshsession.c | 8 +- src/rpc/virnettlscontext.c | 6 +- src/security/security_manager.c | 2 +- src/util/virdnsmasq.c | 2 +- src/util/virfilecache.c | 4 +- src/util/virmdev.c | 2 +- src/util/virnvme.c | 2 +- src/util/virpci.c | 2 +- src/util/virresctrl.c | 40 src/util/virscsi.c | 2 +- src/util/virscsivhost.c | 2 +- src/util/virusb.c | 2 +- tests/virfilecachetest.c| 2 +- 43 files changed, 235 inse
[libvirt PATCH v2 14/24] rpc: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/rpc/virnetmessage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index b728e73f4f..9f7334ae4c 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -88,7 +88,7 @@ void virNetMessageFree(virNetMessagePtr msg) msg->cb(msg, msg->opaque); virNetMessageClearPayload(msg); -VIR_FREE(msg); +g_free(msg); } void virNetMessageQueuePush(virNetMessagePtr *queue, virNetMessagePtr msg) -- 2.29.2
[libvirt PATCH v2 13/24] remote: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/remote/remote_daemon_config.c | 48 ++---
src/remote/remote_daemon_dispatch.c | 4 +--
src/remote/remote_driver.c | 2 +-
3 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/remote/remote_daemon_config.c
b/src/remote/remote_daemon_config.c
index f0cca42925..b354dc4cc0 100644
--- a/src/remote/remote_daemon_config.c
+++ b/src/remote/remote_daemon_config.c
@@ -173,53 +173,53 @@ daemonConfigFree(struct daemonConfig *data)
return;
#ifdef WITH_IP
-VIR_FREE(data->listen_addr);
-VIR_FREE(data->tls_port);
-VIR_FREE(data->tcp_port);
+g_free(data->listen_addr);
+g_free(data->tls_port);
+g_free(data->tcp_port);
#endif /* ! WITH_IP */
tmp = data->access_drivers;
while (tmp && *tmp) {
-VIR_FREE(*tmp);
+g_free(*tmp);
tmp++;
}
-VIR_FREE(data->access_drivers);
+g_free(data->access_drivers);
-VIR_FREE(data->unix_sock_admin_perms);
-VIR_FREE(data->unix_sock_ro_perms);
-VIR_FREE(data->unix_sock_rw_perms);
-VIR_FREE(data->unix_sock_group);
-VIR_FREE(data->unix_sock_dir);
+g_free(data->unix_sock_admin_perms);
+g_free(data->unix_sock_ro_perms);
+g_free(data->unix_sock_rw_perms);
+g_free(data->unix_sock_group);
+g_free(data->unix_sock_dir);
tmp = data->sasl_allowed_username_list;
while (tmp && *tmp) {
-VIR_FREE(*tmp);
+g_free(*tmp);
tmp++;
}
-VIR_FREE(data->sasl_allowed_username_list);
+g_free(data->sasl_allowed_username_list);
#ifdef WITH_IP
tmp = data->tls_allowed_dn_list;
while (tmp && *tmp) {
-VIR_FREE(*tmp);
+g_free(*tmp);
tmp++;
}
-VIR_FREE(data->tls_allowed_dn_list);
+g_free(data->tls_allowed_dn_list);
-VIR_FREE(data->tls_priority);
+g_free(data->tls_priority);
-VIR_FREE(data->key_file);
-VIR_FREE(data->ca_file);
-VIR_FREE(data->cert_file);
-VIR_FREE(data->crl_file);
+g_free(data->key_file);
+g_free(data->ca_file);
+g_free(data->cert_file);
+g_free(data->crl_file);
#endif /* ! WITH_IP */
-VIR_FREE(data->host_uuid);
-VIR_FREE(data->host_uuid_source);
-VIR_FREE(data->log_filters);
-VIR_FREE(data->log_outputs);
+g_free(data->host_uuid);
+g_free(data->host_uuid_source);
+g_free(data->log_filters);
+g_free(data->log_outputs);
-VIR_FREE(data);
+g_free(data);
}
static int
diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
index 46683aa4a7..b7ef1f4b26 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -150,7 +150,7 @@ remoteEventCallbackFree(void *opaque)
return;
virObjectUnref(callback->program);
virObjectUnref(callback->client);
-VIR_FREE(callback);
+g_free(callback);
}
@@ -1742,7 +1742,7 @@ void remoteClientFree(void *data)
if (priv->storageConn)
virConnectClose(priv->storageConn);
-VIR_FREE(priv);
+g_free(priv);
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 1b784e61c7..8d6790ccf2 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5614,7 +5614,7 @@ static void remoteStreamCallbackFree(void *opaque)
(cbdata->ff)(cbdata->opaque);
virObjectUnref(cbdata->st);
-VIR_FREE(opaque);
+g_free(opaque);
}
--
2.29.2
[libvirt PATCH v2 15/24] security: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/security/security_dac.c | 8
src/security/security_selinux.c | 10 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 389d1dac51..00eeae0d27 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -141,12 +141,12 @@ virSecurityDACChownListFree(void *opaque)
return;
for (i = 0; i < list->nItems; i++) {
-VIR_FREE(list->items[i]->path);
-VIR_FREE(list->items[i]);
+g_free(list->items[i]->path);
+g_free(list->items[i]);
}
-VIR_FREE(list->items);
+g_free(list->items);
virObjectUnref(list->manager);
-VIR_FREE(list);
+g_free(list);
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 8cc30a0e72..1d1d9edfff 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -108,9 +108,9 @@
virSecuritySELinuxContextItemFree(virSecuritySELinuxContextItemPtr item)
if (!item)
return;
-VIR_FREE(item->path);
-VIR_FREE(item->tcon);
-VIR_FREE(item);
+g_free(item->path);
+g_free(item->tcon);
+g_free(item);
}
static int
@@ -152,9 +152,9 @@ virSecuritySELinuxContextListFree(void *opaque)
for (i = 0; i < list->nItems; i++)
virSecuritySELinuxContextItemFree(list->items[i]);
-VIR_FREE(list->items);
+g_free(list->items);
virObjectUnref(list->manager);
-VIR_FREE(list);
+g_free(list);
}
--
2.29.2
[libvirt PATCH v2 21/24] qemu: rename virFirmware*Free() functions to have more accurate names
Several functions had the names virFirmware[something]Free(), but they
aren't taking a pointer to some object and freeing it. Instead, they
are making a copy of the content of an entire object, then Freeing the
objects pointed to by that content.
As a first step in a too-complicated cleanup just to eliminate a few
occurrences of VIR_FREE(), this patch renames those functions to more
accurately reflect what they do - they Free the *Content* of their
arguments.
Signed-off-by: Laine Stump
---
src/qemu/qemu_firmware.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index fe30db4f79..c22b1f1e9c 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -188,7 +188,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuFirmwareOSInterface,
qemuFirmwareOSInterfaceFr
static void
-qemuFirmwareFlashFileFree(qemuFirmwareFlashFile flash)
+qemuFirmwareFlashFileFreeContent(qemuFirmwareFlashFile flash)
{
VIR_FREE(flash.filename);
VIR_FREE(flash.format);
@@ -196,39 +196,39 @@ qemuFirmwareFlashFileFree(qemuFirmwareFlashFile flash)
static void
-qemuFirmwareMappingFlashFree(qemuFirmwareMappingFlash flash)
+qemuFirmwareMappingFlashFreeContent(qemuFirmwareMappingFlash flash)
{
-qemuFirmwareFlashFileFree(flash.executable);
-qemuFirmwareFlashFileFree(flash.nvram_template);
+qemuFirmwareFlashFileFreeContent(flash.executable);
+qemuFirmwareFlashFileFreeContent(flash.nvram_template);
}
static void
-qemuFirmwareMappingKernelFree(qemuFirmwareMappingKernel kernel)
+qemuFirmwareMappingKernelFreeContent(qemuFirmwareMappingKernel kernel)
{
VIR_FREE(kernel.filename);
}
static void
-qemuFirmwareMappingMemoryFree(qemuFirmwareMappingMemory memory)
+qemuFirmwareMappingMemoryFreeContent(qemuFirmwareMappingMemory memory)
{
VIR_FREE(memory.filename);
}
static void
-qemuFirmwareMappingFree(qemuFirmwareMapping mapping)
+qemuFirmwareMappingFreeContent(qemuFirmwareMapping mapping)
{
switch (mapping.device) {
case QEMU_FIRMWARE_DEVICE_FLASH:
-qemuFirmwareMappingFlashFree(mapping.data.flash);
+qemuFirmwareMappingFlashFreeContent(mapping.data.flash);
break;
case QEMU_FIRMWARE_DEVICE_KERNEL:
-qemuFirmwareMappingKernelFree(mapping.data.kernel);
+qemuFirmwareMappingKernelFreeContent(mapping.data.kernel);
break;
case QEMU_FIRMWARE_DEVICE_MEMORY:
-qemuFirmwareMappingMemoryFree(mapping.data.memory);
+qemuFirmwareMappingMemoryFreeContent(mapping.data.memory);
break;
case QEMU_FIRMWARE_DEVICE_NONE:
case QEMU_FIRMWARE_DEVICE_LAST:
@@ -271,7 +271,7 @@ qemuFirmwareFree(qemuFirmwarePtr fw)
return;
qemuFirmwareOSInterfaceFree(fw->interfaces);
-qemuFirmwareMappingFree(fw->mapping);
+qemuFirmwareMappingFreeContent(fw->mapping);
for (i = 0; i < fw->ntargets; i++)
qemuFirmwareTargetFree(fw->targets[i]);
g_free(fw->targets);
--
2.29.2
[libvirt PATCH v2 17/24] tests: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
tests/nodedevmdevctltest.c | 4 ++--
tests/nwfilterxml2firewalltest.c | 2 +-
tests/qemuhotplugtest.c | 12 ++--
tests/qemumonitortestutils.c | 28 ++--
tests/virnetdaemontest.c | 2 +-
tests/virnetserverclienttest.c | 2 +-
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index dab4b487b9..1bad65549b 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -152,8 +152,8 @@ nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv)
virNodeDeviceObjListFree(drv->devs);
virCondDestroy(>initCond);
virMutexDestroy(>lock);
-VIR_FREE(drv->stateDir);
-VIR_FREE(drv);
+g_free(drv->stateDir);
+g_free(drv);
}
/* Add a fake root 'computer' device */
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index 0901250aaf..3e2ab0b0ba 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -174,7 +174,7 @@ virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst)
return;
virHashFree(inst->vars);
-VIR_FREE(inst);
+g_free(inst);
}
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 776fb019f3..df5c9c9059 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -383,12 +383,12 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData
*data)
if (!data)
return;
-VIR_FREE(data->file_xml_dom);
-VIR_FREE(data->file_xml_res_live);
-VIR_FREE(data->file_xml_res_conf);
-VIR_FREE(data->file_json_monitor);
+g_free(data->file_xml_dom);
+g_free(data->file_xml_res_live);
+g_free(data->file_xml_res_conf);
+g_free(data->file_json_monitor);
-VIR_FREE(data->xml_dom);
+g_free(data->xml_dom);
if (data->vm) {
priv = data->vm->privateData;
@@ -402,7 +402,7 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData
*data)
virObjectLock(mon);
qemuMonitorTestFree(data->mon);
}
-VIR_FREE(data);
+g_free(data);
}
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c4f7082655..ae3fcf9311 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -99,7 +99,7 @@ qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)
if (item->freecb)
(item->freecb)(item->opaque);
-VIR_FREE(item);
+g_free(item);
}
@@ -423,8 +423,8 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
if (timer != -1)
virEventRemoveTimeout(timer);
-VIR_FREE(test->incoming);
-VIR_FREE(test->outgoing);
+g_free(test->incoming);
+g_free(test->outgoing);
for (i = 0; i < test->nitems; i++) {
if (!test->allowUnusedCommands) {
@@ -435,12 +435,12 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
qemuMonitorTestItemFree(test->items[i]);
}
-VIR_FREE(test->items);
+g_free(test->items);
if (test->tmpdir && rmdir(test->tmpdir) < 0)
VIR_WARN("Failed to remove tempdir: %s", g_strerror(errno));
-VIR_FREE(test->tmpdir);
+g_free(test->tmpdir);
if (!test->allowUnusedCommands &&
test->nitems != 0) {
@@ -448,7 +448,7 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
}
virMutexDestroy(>lock);
-VIR_FREE(test);
+g_free(test);
}
@@ -518,16 +518,16 @@ qemuMonitorTestHandlerDataFree(void *opaque)
return;
for (i = 0; i < data->nargs; i++) {
-VIR_FREE(data->args[i].argname);
-VIR_FREE(data->args[i].argval);
+g_free(data->args[i].argname);
+g_free(data->args[i].argval);
}
-VIR_FREE(data->command_name);
-VIR_FREE(data->cmderr);
-VIR_FREE(data->response);
-VIR_FREE(data->args);
-VIR_FREE(data->expectArgs);
-VIR_FREE(data);
+g_free(data->command_name);
+g_free(data->cmderr);
+g_free(data->response);
+g_free(data->args);
+g_free(data->expectArgs);
+g_free(data);
}
diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c
index a20f351f93..fb40fe9b80 100644
--- a/tests/virnetdaemontest.c
+++ b/tests/virnetdaemontest.c
@@ -76,7 +76,7 @@ testClientNewPostExec(virNetServerClientPtr client,
static void
testClientFree(void *opaque)
{
-VIR_FREE(opaque);
+g_free(opaque);
}
diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c
index a4341c1641..959100ec66 100644
--- a/tests/virnetserverclienttest.c
+++ b/tests/virnetserverclienttest.c
@@ -41,7 +41,7 @@ testClientNew(virNetServerClientPtr client G_GNUC_UNUSED,
static void
testClientFree(void *opaque)
{
-VIR_FREE(opaque);
+g_free(opaque);
}
static int testIdentity(const void *opaque G_GNUC_UNUSED)
--
2.29.2
[libvirt PATCH v2 11/24] locking: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/locking/lock_daemon.c | 6 +++---
src/locking/lock_daemon_config.c | 6 +++---
src/locking/lock_driver_lockd.c | 10 +-
src/locking/lock_driver_sanlock.c | 6 +++---
src/locking/lock_manager.c| 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 851e9fc6f0..94fe374df6 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -92,7 +92,7 @@ virLockDaemonFree(virLockDaemonPtr lockd)
virHashFree(lockd->lockspaces);
virLockSpaceFree(lockd->defaultLockspace);
-VIR_FREE(lockd);
+g_free(lockd);
}
static inline void
@@ -435,8 +435,8 @@ virLockDaemonClientFree(void *opaque)
}
g_mutex_clear(>lock);
-VIR_FREE(priv->ownerName);
-VIR_FREE(priv);
+g_free(priv->ownerName);
+g_free(priv);
}
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index 6346703715..f693911997 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -71,10 +71,10 @@ virLockDaemonConfigFree(virLockDaemonConfigPtr data)
if (!data)
return;
-VIR_FREE(data->log_filters);
-VIR_FREE(data->log_outputs);
+g_free(data->log_filters);
+g_free(data->log_outputs);
-VIR_FREE(data);
+g_free(data);
}
static int
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 5ebefa48d0..a1deb55788 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -369,13 +369,13 @@
virLockManagerLockDaemonPrivateFree(virLockManagerLockDaemonPrivatePtr priv)
return;
for (i = 0; i < priv->nresources; i++) {
-VIR_FREE(priv->resources[i].lockspace);
-VIR_FREE(priv->resources[i].name);
+g_free(priv->resources[i].lockspace);
+g_free(priv->resources[i].name);
}
-VIR_FREE(priv->resources);
+g_free(priv->resources);
-VIR_FREE(priv->name);
-VIR_FREE(priv);
+g_free(priv->name);
+g_free(priv);
}
static void virLockManagerLockDaemonFree(virLockManagerPtr lock)
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index 7319f56819..aaffe30e6f 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -513,10 +513,10 @@ static void virLockManagerSanlockFree(virLockManagerPtr
lock)
if (!priv)
return;
-VIR_FREE(priv->vm_name);
+g_free(priv->vm_name);
for (i = 0; i < priv->res_count; i++)
-VIR_FREE(priv->res_args[i]);
-VIR_FREE(priv);
+g_free(priv->res_args[i]);
+g_free(priv);
lock->privateData = NULL;
}
diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
index d83a192bf6..db724eb30f 100644
--- a/src/locking/lock_manager.c
+++ b/src/locking/lock_manager.c
@@ -387,7 +387,7 @@ int virLockManagerFree(virLockManagerPtr lock)
lock->driver->drvFree(lock);
-VIR_FREE(lock);
+g_free(lock);
return 0;
}
--
2.29.2
[libvirt PATCH v2 24/24] rpc: rename virNetSessionAuthMethodsFree to virNetSessionAuthMethodsClear
This is another *Free() function that doesn't free the object it is
passed. Instead it frees and clears some parts of the object.
In this case, the function is actually called from two places, and one
of them (virNetSSHSessionAuthReset) appears to be assuming that the
pointers actually *will* be cleared. So the proper thing to do here
(?) is to rename the function to virNetSSHSesionAuthMethodsClear().
(NB: virNetSSHSessionAuthReset is seemingly never called from
anywhere. Is this one of those functions that actually *is* called by
some strange MACRO invocation? Or it is truly one of those
"written-but-never-used" functions that can be deleted? (if the latter
is the case, then I would rather move the contents of
virNetSessionAuthMethodsFree() into its only other caller,
virNetSSHSessionDispose(), so that the VIR_FREEs could be replaced
with g_free.)
Signed-off-by: Laine Stump
---
src/rpc/virnetsshsession.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
index 4b56363fa0..5bfe311544 100644
--- a/src/rpc/virnetsshsession.c
+++ b/src/rpc/virnetsshsession.c
@@ -113,7 +113,7 @@ struct _virNetSSHSession {
};
static void
-virNetSSHSessionAuthMethodsFree(virNetSSHSessionPtr sess)
+virNetSSHSessionAuthMethodsClear(virNetSSHSessionPtr sess)
{
size_t i;
@@ -149,7 +149,7 @@ virNetSSHSessionDispose(void *obj)
libssh2_session_free(sess->session);
}
-virNetSSHSessionAuthMethodsFree(sess);
+virNetSSHSessionAuthMethodsClear(sess);
VIR_FREE(sess->channelCommand);
VIR_FREE(sess->hostname);
@@ -971,7 +971,7 @@ void
virNetSSHSessionAuthReset(virNetSSHSessionPtr sess)
{
virObjectLock(sess);
-virNetSSHSessionAuthMethodsFree(sess);
+virNetSSHSessionAuthMethodsClear(sess);
virObjectUnlock(sess);
}
--
2.29.2
[libvirt PATCH v2 22/24] qemu: pass pointers instead of copying objects for qemuFirmware*FreeContent()
These functions all cooperate to free memory pointed to by a single
object that contains (doesn't *point to*, but acutally contains)
several sub-objects. They were written to send copies of these
sub-objects to subordinate functions, rather than just sending
pointers to the sub-objects.
Let's change these functions to just send pointers to the objects
they're cleaning out rather than all the wasteful and pointless
copying.
Signed-off-by: Laine Stump
---
src/qemu/qemu_firmware.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index c22b1f1e9c..aad39ee038 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -188,47 +188,47 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuFirmwareOSInterface,
qemuFirmwareOSInterfaceFr
static void
-qemuFirmwareFlashFileFreeContent(qemuFirmwareFlashFile flash)
+qemuFirmwareFlashFileFreeContent(qemuFirmwareFlashFilePtr flash)
{
-VIR_FREE(flash.filename);
-VIR_FREE(flash.format);
+VIR_FREE(flash->filename);
+VIR_FREE(flash->format);
}
static void
-qemuFirmwareMappingFlashFreeContent(qemuFirmwareMappingFlash flash)
+qemuFirmwareMappingFlashFreeContent(qemuFirmwareMappingFlashPtr flash)
{
-qemuFirmwareFlashFileFreeContent(flash.executable);
-qemuFirmwareFlashFileFreeContent(flash.nvram_template);
+qemuFirmwareFlashFileFreeContent(>executable);
+qemuFirmwareFlashFileFreeContent(>nvram_template);
}
static void
-qemuFirmwareMappingKernelFreeContent(qemuFirmwareMappingKernel kernel)
+qemuFirmwareMappingKernelFreeContent(qemuFirmwareMappingKernelPtr kernel)
{
-VIR_FREE(kernel.filename);
+VIR_FREE(kernel->filename);
}
static void
-qemuFirmwareMappingMemoryFreeContent(qemuFirmwareMappingMemory memory)
+qemuFirmwareMappingMemoryFreeContent(qemuFirmwareMappingMemoryPtr memory)
{
-VIR_FREE(memory.filename);
+VIR_FREE(memory->filename);
}
static void
-qemuFirmwareMappingFreeContent(qemuFirmwareMapping mapping)
+qemuFirmwareMappingFreeContent(qemuFirmwareMappingPtr mapping)
{
-switch (mapping.device) {
+switch (mapping->device) {
case QEMU_FIRMWARE_DEVICE_FLASH:
-qemuFirmwareMappingFlashFreeContent(mapping.data.flash);
+qemuFirmwareMappingFlashFreeContent(>data.flash);
break;
case QEMU_FIRMWARE_DEVICE_KERNEL:
-qemuFirmwareMappingKernelFreeContent(mapping.data.kernel);
+qemuFirmwareMappingKernelFreeContent(>data.kernel);
break;
case QEMU_FIRMWARE_DEVICE_MEMORY:
-qemuFirmwareMappingMemoryFreeContent(mapping.data.memory);
+qemuFirmwareMappingMemoryFreeContent(>data.memory);
break;
case QEMU_FIRMWARE_DEVICE_NONE:
case QEMU_FIRMWARE_DEVICE_LAST:
@@ -271,7 +271,7 @@ qemuFirmwareFree(qemuFirmwarePtr fw)
return;
qemuFirmwareOSInterfaceFree(fw->interfaces);
-qemuFirmwareMappingFreeContent(fw->mapping);
+qemuFirmwareMappingFreeContent(>mapping);
for (i = 0; i < fw->ntargets; i++)
qemuFirmwareTargetFree(fw->targets[i]);
g_free(fw->targets);
--
2.29.2
[libvirt PATCH v2 20/24] util: rename two *Free() functions while changing VIR_FREE to g_free
dhcpHostFree() and addnHostFree() don't follow the normal pattern of
*Free functions in the rest of libvirt code - they are actually more
similar to the *Dispose() functions, in that they free all subordinate
objects, but not the object pointed to by the argument
itself. However, the arguments aren't virObjects, so it wouldn't be
proper to name them *Dispose() either.
They *currently* behave similar to a *Clear() function, in that they
free all the subordinate objects and nullify the pointers of those
objects. HOWEVER, we don't actually need or want that behavior - the
two functions in question are only called as part of a higher level
*Free() function, and the pointers are not referenced in any way
between the time they are freed and when the parent object is freed.
So, since the current name isn't correct, nor is *Dispose(), and we
want to change the behavior in such a way that *Clear() also wouldn't
be correct, lets name the functions *FreeContent(), which is an
accurate description of what the functions do, and what we *want* them
to do.
And since it's such a small patch, we can go ahead and change that
behavior - replacing the VIR_FREEs with g_free.
Signed-off-by: Laine Stump
---
src/util/virdnsmasq.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index c21819c981..653d46bef9 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -50,20 +50,20 @@ VIR_LOG_INIT("util.dnsmasq");
#define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
static void
-dhcphostFree(dnsmasqDhcpHost *host)
+dhcphostFreeContent(dnsmasqDhcpHost *host)
{
-VIR_FREE(host->host);
+g_free(host->host);
}
static void
-addnhostFree(dnsmasqAddnHost *host)
+addnhostFreeContent(dnsmasqAddnHost *host)
{
size_t i;
for (i = 0; i < host->nhostnames; i++)
-VIR_FREE(host->hostnames[i]);
-VIR_FREE(host->hostnames);
-VIR_FREE(host->ip);
+g_free(host->hostnames[i]);
+g_free(host->hostnames);
+g_free(host->ip);
}
static void
@@ -73,7 +73,7 @@ addnhostsFree(dnsmasqAddnHostsfile *addnhostsfile)
if (addnhostsfile->hosts) {
for (i = 0; i < addnhostsfile->nhosts; i++)
-addnhostFree(>hosts[i]);
+addnhostFreeContent(>hosts[i]);
g_free(addnhostsfile->hosts);
@@ -270,7 +270,7 @@ hostsfileFree(dnsmasqHostsfile *hostsfile)
if (hostsfile->hosts) {
for (i = 0; i < hostsfile->nhosts; i++)
-dhcphostFree(>hosts[i]);
+dhcphostFreeContent(>hosts[i]);
g_free(hostsfile->hosts);
--
2.29.2
[libvirt PATCH v2 23/24] qemu: replace VIR_FREE with g_free in qemuFirmware*FreeContent()
These functions are all only called as a part of qemuFirmwareFree(),
which frees the qemuFirmware object before return, so we can be sure
none of the pointers is referenced after freeing (and thus there is no
need to clear any of them).
Signed-off-by: Laine Stump
---
src/qemu/qemu_firmware.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index aad39ee038..e6541d505e 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -190,8 +190,8 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuFirmwareOSInterface,
qemuFirmwareOSInterfaceFr
static void
qemuFirmwareFlashFileFreeContent(qemuFirmwareFlashFilePtr flash)
{
-VIR_FREE(flash->filename);
-VIR_FREE(flash->format);
+g_free(flash->filename);
+g_free(flash->format);
}
@@ -206,14 +206,14 @@
qemuFirmwareMappingFlashFreeContent(qemuFirmwareMappingFlashPtr flash)
static void
qemuFirmwareMappingKernelFreeContent(qemuFirmwareMappingKernelPtr kernel)
{
-VIR_FREE(kernel->filename);
+g_free(kernel->filename);
}
static void
qemuFirmwareMappingMemoryFreeContent(qemuFirmwareMappingMemoryPtr memory)
{
-VIR_FREE(memory->filename);
+g_free(memory->filename);
}
--
2.29.2
[libvirt PATCH v2 19/24] libvirtd: replace straggler VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/libvirt-domain.c | 18 +-
src/libvirt-network.c | 14 +++---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index c9f8ffdb56..dba89a7d3a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -7789,8 +7789,8 @@ virDomainIOThreadInfoFree(virDomainIOThreadInfoPtr info)
if (!info)
return;
-VIR_FREE(info->cpumap);
-VIR_FREE(info);
+g_free(info->cpumap);
+g_free(info);
}
@@ -12084,10 +12084,10 @@ virDomainStatsRecordListFree(virDomainStatsRecordPtr
*stats)
for (next = stats; *next; next++) {
virTypedParamsFree((*next)->params, (*next)->nparams);
virDomainFree((*next)->dom);
-VIR_FREE(*next);
+g_free(*next);
}
-VIR_FREE(stats);
+g_free(stats);
}
@@ -12281,14 +12281,14 @@ virDomainInterfaceFree(virDomainInterfacePtr iface)
if (!iface)
return;
-VIR_FREE(iface->name);
-VIR_FREE(iface->hwaddr);
+g_free(iface->name);
+g_free(iface->hwaddr);
for (i = 0; i < iface->naddrs; i++)
-VIR_FREE(iface->addrs[i].addr);
-VIR_FREE(iface->addrs);
+g_free(iface->addrs[i].addr);
+g_free(iface->addrs);
-VIR_FREE(iface);
+g_free(iface);
}
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index f691b672c7..92bdbacbff 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -1240,13 +1240,13 @@ virNetworkDHCPLeaseFree(virNetworkDHCPLeasePtr lease)
{
if (!lease)
return;
-VIR_FREE(lease->iface);
-VIR_FREE(lease->mac);
-VIR_FREE(lease->iaid);
-VIR_FREE(lease->ipaddr);
-VIR_FREE(lease->hostname);
-VIR_FREE(lease->clientid);
-VIR_FREE(lease);
+g_free(lease->iface);
+g_free(lease->mac);
+g_free(lease->iaid);
+g_free(lease->ipaddr);
+g_free(lease->hostname);
+g_free(lease->clientid);
+g_free(lease);
}
--
2.29.2
[libvirt PATCH v2 16/24] tools: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
tools/virsh-checkpoint.c | 6 +++---
tools/virsh-domain-monitor.c | 4 ++--
tools/virsh-domain.c | 2 +-
tools/virsh-interface.c | 4 ++--
tools/virsh-network.c| 8
tools/virsh-nodedev.c| 4 ++--
tools/virsh-nwfilter.c | 8
tools/virsh-pool.c | 4 ++--
tools/virsh-secret.c | 4 ++--
tools/virsh-snapshot.c | 6 +++---
tools/virsh-volume.c | 4 ++--
tools/vsh-table.c| 10 +-
tools/vsh.c | 6 +++---
13 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
index f6396d16eb..6e68ba66ff 100644
--- a/tools/virsh-checkpoint.c
+++ b/tools/virsh-checkpoint.c
@@ -552,11 +552,11 @@ virshCheckpointListFree(virshCheckpointListPtr
checkpointlist)
if (checkpointlist->chks) {
for (i = 0; i < checkpointlist->nchks; i++) {
virshDomainCheckpointFree(checkpointlist->chks[i].chk);
-VIR_FREE(checkpointlist->chks[i].parent);
+g_free(checkpointlist->chks[i].parent);
}
-VIR_FREE(checkpointlist->chks);
+g_free(checkpointlist->chks);
}
-VIR_FREE(checkpointlist);
+g_free(checkpointlist);
}
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 5d0a03afa9..02ff1fbd62 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1609,9 +1609,9 @@ virshDomainListFree(virshDomainListPtr domlist)
if (domlist && domlist->domains) {
for (i = 0; i < domlist->ndomains; i++)
virshDomainFree(domlist->domains[i]);
-VIR_FREE(domlist->domains);
+g_free(domlist->domains);
}
-VIR_FREE(domlist);
+g_free(domlist);
}
static virshDomainListPtr
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fb8b4bdb72..98a87dd43c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1796,7 +1796,7 @@ virshBlockJobWaitFree(virshBlockJobWaitDataPtr data)
if (data->cb_id2 >= 0)
virConnectDomainEventDeregisterAny(priv->conn, data->cb_id2);
-VIR_FREE(data);
+g_free(data);
}
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 858052d341..a11d4ba607 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -176,9 +176,9 @@ virshInterfaceListFree(virshInterfaceListPtr list)
if (list->ifaces[i])
virInterfaceFree(list->ifaces[i]);
}
-VIR_FREE(list->ifaces);
+g_free(list->ifaces);
}
-VIR_FREE(list);
+g_free(list);
}
static virshInterfaceListPtr
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 9e3b31a60c..a203fa64ff 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -459,9 +459,9 @@ virshNetworkListFree(virshNetworkListPtr list)
if (list->nets[i])
virNetworkFree(list->nets[i]);
}
-VIR_FREE(list->nets);
+g_free(list->nets);
}
-VIR_FREE(list);
+g_free(list);
}
static virshNetworkListPtr
@@ -1676,9 +1676,9 @@ virshNetworkPortListFree(virshNetworkPortListPtr list)
if (list->ports[i])
virNetworkPortFree(list->ports[i]);
}
-VIR_FREE(list->ports);
+g_free(list->ports);
}
-VIR_FREE(list);
+g_free(list);
}
static virshNetworkPortListPtr
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 69422e20f5..fedc8497f8 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -205,9 +205,9 @@ virshNodeDeviceListFree(virshNodeDeviceListPtr list)
if (list->devices[i])
virNodeDeviceFree(list->devices[i]);
}
-VIR_FREE(list->devices);
+g_free(list->devices);
}
-VIR_FREE(list);
+g_free(list);
}
static virshNodeDeviceListPtr
diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c
index 768a96e949..0d09a016bc 100644
--- a/tools/virsh-nwfilter.c
+++ b/tools/virsh-nwfilter.c
@@ -234,9 +234,9 @@ virshNWFilterListFree(virshNWFilterListPtr list)
if (list->filters[i])
virNWFilterFree(list->filters[i]);
}
-VIR_FREE(list->filters);
+g_free(list->filters);
}
-VIR_FREE(list);
+g_free(list);
}
static virshNWFilterListPtr
@@ -654,9 +654,9 @@ virshNWFilterBindingListFree(virshNWFilterBindingListPtr
list)
if (list->bindings[i])
virNWFilterBindingFree(list->bindings[i]);
}
-VIR_FREE(list->bindings);
+g_free(list->bindings);
}
-VIR_FREE(list);
+g_free(list);
}
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 7835fa6d75..5256b50589 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -850,9
[libvirt PATCH v2 18/24] storage: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/storage/storage_backend_fs.c | 6 +++---
src/storage/storage_backend_rbd.c | 10 +-
src/storage/storage_backend_scsi.c | 4 ++--
src/storage/storage_driver.c | 4 ++--
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index b98d06c644..48924167eb 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -551,10 +551,10 @@ virStoragePoolDefFSNamespaceFree(void *nsdata)
return;
for (i = 0; i < cmdopts->noptions; i++)
-VIR_FREE(cmdopts->options[i]);
-VIR_FREE(cmdopts->options);
+g_free(cmdopts->options[i]);
+g_free(cmdopts->options);
-VIR_FREE(cmdopts);
+g_free(cmdopts);
}
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 007c53f7ac..2804759557 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -68,13 +68,13 @@ virStoragePoolDefRBDNamespaceFree(void *nsdata)
return;
for (i = 0; i < cmdopts->noptions; i++) {
-VIR_FREE(cmdopts->names[i]);
-VIR_FREE(cmdopts->values[i]);
+g_free(cmdopts->names[i]);
+g_free(cmdopts->values[i]);
}
-VIR_FREE(cmdopts->names);
-VIR_FREE(cmdopts->values);
+g_free(cmdopts->names);
+g_free(cmdopts->values);
-VIR_FREE(cmdopts);
+g_free(cmdopts);
}
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 69a01d1a24..9474c60c72 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -95,8 +95,8 @@ virStoragePoolFCRefreshDataFree(void *opaque)
{
virStoragePoolFCRefreshInfoPtr cbdata = opaque;
-VIR_FREE(cbdata->fchost_name);
-VIR_FREE(cbdata);
+g_free(cbdata->fchost_name);
+g_free(cbdata);
}
/**
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 16bc53aa46..9c5be071df 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2268,8 +2268,8 @@ virStorageVolPoolRefreshDataFree(void *opaque)
{
virStorageVolStreamInfoPtr cbdata = opaque;
-VIR_FREE(cbdata->pool_name);
-VIR_FREE(cbdata);
+g_free(cbdata->pool_name);
+g_free(cbdata);
}
static int
--
2.29.2
[libvirt PATCH v2 09/24] vz: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/vz/vz_driver.c | 8 src/vz/vz_utils.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index b60e99d4f5..0ebcb06234 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -2729,10 +2729,10 @@ vzMigrationCookieFree(vzMigrationCookiePtr mig) if (!mig) return; -VIR_FREE(mig->session_uuid); -VIR_FREE(mig->uuid); -VIR_FREE(mig->name); -VIR_FREE(mig); +g_free(mig->session_uuid); +g_free(mig->uuid); +g_free(mig->name); +g_free(mig); } static int diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 4a79b55f3e..e5106619b6 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -582,7 +582,7 @@ vzDomObjFree(void* p) PrlHandle_Free(pdom->sdkdom); PrlHandle_Free(pdom->stats); virCondDestroy(>job.cond); -VIR_FREE(pdom); +g_free(pdom); }; #define VZ_JOB_WAIT_TIME (1000 * 30) -- 2.29.2
[libvirt PATCH v2 08/24] vmx: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/vmx/vmx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 56318fc8b2..db535ba260 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -650,10 +650,10 @@ virVMXDomainDefNamespaceFree(void *nsdata)
struct virVMXDomainDefNamespaceData *data = nsdata;
if (data) {
-VIR_FREE(data->datacenterPath);
-VIR_FREE(data->moref);
+g_free(data->datacenterPath);
+g_free(data->moref);
}
-VIR_FREE(data);
+g_free(data);
}
static int
--
2.29.2
[libvirt PATCH v2 12/24] logging: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/logging/log_daemon.c| 4 ++-- src/logging/log_daemon_config.c | 6 +++--- src/logging/log_handler.c | 6 +++--- src/logging/log_manager.c | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 6b8f3b6fe5..770f6dd273 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -89,7 +89,7 @@ virLogDaemonFree(virLogDaemonPtr logd) g_mutex_clear(>lock); virObjectUnref(logd->dmn); -VIR_FREE(logd); +g_free(logd); } @@ -314,7 +314,7 @@ virLogDaemonClientFree(void *opaque) (unsigned long long)priv->clientPid); g_mutex_clear(>lock); -VIR_FREE(priv); +g_free(priv); } diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index 5577991b13..075cf766ec 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -74,10 +74,10 @@ virLogDaemonConfigFree(virLogDaemonConfigPtr data) if (!data) return; -VIR_FREE(data->log_filters); -VIR_FREE(data->log_outputs); +g_free(data->log_filters); +g_free(data->log_outputs); -VIR_FREE(data); +g_free(data); } static int diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 95d909b44e..45a4763525 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -98,9 +98,9 @@ virLogHandlerLogFileFree(virLogHandlerLogFilePtr file) if (file->watch != -1) virEventRemoveHandle(file->watch); -VIR_FREE(file->driver); -VIR_FREE(file->domname); -VIR_FREE(file); +g_free(file->driver); +g_free(file->domname); +g_free(file); } diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c index 57be340a2d..56927c1d6d 100644 --- a/src/logging/log_manager.c +++ b/src/logging/log_manager.c @@ -137,7 +137,7 @@ virLogManagerFree(virLogManagerPtr mgr) virObjectUnref(mgr->program); virObjectUnref(mgr->client); -VIR_FREE(mgr); +g_free(mgr); } -- 2.29.2
[libvirt PATCH v2 03/24] bhyve: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/bhyve/bhyve_conf.c | 6 +++---
src/bhyve/bhyve_domain.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c
index f3e2ebf7c7..9012eb8b5f 100644
--- a/src/bhyve/bhyve_conf.c
+++ b/src/bhyve/bhyve_conf.c
@@ -109,8 +109,8 @@ bhyveDomainCmdlineDefFree(bhyveDomainCmdlineDefPtr def)
return;
for (i = 0; i < def->num_args; i++)
-VIR_FREE(def->args[i]);
+g_free(def->args[i]);
-VIR_FREE(def->args);
-VIR_FREE(def);
+g_free(def->args);
+g_free(def);
}
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index f0e553113f..1d99ba6c96 100644
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -50,7 +50,7 @@ bhyveDomainObjPrivateFree(void *data)
virDomainPCIAddressSetFree(priv->pciaddrs);
-VIR_FREE(priv);
+g_free(priv);
}
virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = {
--
2.29.2
[libvirt PATCH v2 07/24] vbox: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/vbox/vbox_snapshot_conf.c | 54 +-- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index cb201e777e..f1cae3039a 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -502,14 +502,14 @@ virVboxSnapshotConfHardDiskFree(virVBoxSnapshotConfHardDiskPtr disk) if (!disk) return; -VIR_FREE(disk->uuid); -VIR_FREE(disk->location); -VIR_FREE(disk->format); -VIR_FREE(disk->type); +g_free(disk->uuid); +g_free(disk->location); +g_free(disk->format); +g_free(disk->type); for (i = 0; i < disk->nchildren; i++) virVboxSnapshotConfHardDiskFree(disk->children[i]); -VIR_FREE(disk->children); -VIR_FREE(disk); +g_free(disk->children); +g_free(disk); } @@ -523,11 +523,11 @@ virVBoxSnapshotConfMediaRegistryFree(virVBoxSnapshotConfMediaRegistryPtr mediaRe for (i = 0; i < mediaRegistry->ndisks; i++) virVboxSnapshotConfHardDiskFree(mediaRegistry->disks[i]); -VIR_FREE(mediaRegistry->disks); +g_free(mediaRegistry->disks); for (i = 0; i < mediaRegistry->notherMedia; i++) -VIR_FREE(mediaRegistry->otherMedia[i]); -VIR_FREE(mediaRegistry->otherMedia); -VIR_FREE(mediaRegistry); +g_free(mediaRegistry->otherMedia[i]); +g_free(mediaRegistry->otherMedia); +g_free(mediaRegistry); } void @@ -538,16 +538,16 @@ virVBoxSnapshotConfSnapshotFree(virVBoxSnapshotConfSnapshotPtr snapshot) if (!snapshot) return; -VIR_FREE(snapshot->uuid); -VIR_FREE(snapshot->name); -VIR_FREE(snapshot->timeStamp); -VIR_FREE(snapshot->description); -VIR_FREE(snapshot->hardware); -VIR_FREE(snapshot->storageController); +g_free(snapshot->uuid); +g_free(snapshot->name); +g_free(snapshot->timeStamp); +g_free(snapshot->description); +g_free(snapshot->hardware); +g_free(snapshot->storageController); for (i = 0; i < snapshot->nchildren; i++) virVBoxSnapshotConfSnapshotFree(snapshot->children[i]); -VIR_FREE(snapshot->children); -VIR_FREE(snapshot); +g_free(snapshot->children); +g_free(snapshot); } void @@ -556,17 +556,17 @@ virVBoxSnapshotConfMachineFree(virVBoxSnapshotConfMachinePtr machine) if (!machine) return; -VIR_FREE(machine->uuid); -VIR_FREE(machine->name); -VIR_FREE(machine->currentSnapshot); -VIR_FREE(machine->snapshotFolder); -VIR_FREE(machine->lastStateChange); +g_free(machine->uuid); +g_free(machine->name); +g_free(machine->currentSnapshot); +g_free(machine->snapshotFolder); +g_free(machine->lastStateChange); virVBoxSnapshotConfMediaRegistryFree(machine->mediaRegistry); -VIR_FREE(machine->hardware); -VIR_FREE(machine->extraData); +g_free(machine->hardware); +g_free(machine->extraData); virVBoxSnapshotConfSnapshotFree(machine->snapshot); -VIR_FREE(machine->storageController); -VIR_FREE(machine); +g_free(machine->storageController); +g_free(machine); } #define VBOX_SETTINGS_NS "http://www.innotek.de/VirtualBox-settings; -- 2.29.2
[libvirt PATCH v2 10/24] admin: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/admin/admin_server_dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c index 0efb4485d4..57e8904f4c 100644 --- a/src/admin/admin_server_dispatch.c +++ b/src/admin/admin_server_dispatch.c @@ -57,7 +57,7 @@ remoteAdmClientFree(void *data) virMutexDestroy(>lock); virObjectUnref(priv->dmn); -VIR_FREE(priv); +g_free(priv); } void * -- 2.29.2
[libvirt PATCH v2 06/24] test_driver: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/test/test_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 29c4c86b1d..bca1297d1d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -198,8 +198,8 @@ testDomainDefNamespaceFree(void *data)
for (i = 0; i < nsdata->num_snap_nodes; i++)
xmlFreeNode(nsdata->snap_nodes[i]);
-VIR_FREE(nsdata->snap_nodes);
-VIR_FREE(nsdata);
+g_free(nsdata->snap_nodes);
+g_free(nsdata);
}
static int
@@ -428,7 +428,7 @@ static void
testDomainObjPrivateFree(void *data)
{
testDomainObjPrivatePtr priv = data;
-VIR_FREE(priv);
+g_free(priv);
}
--
2.29.2
[libvirt PATCH v2 05/24] qemu: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/qemu/qemu_block.c| 36
src/qemu/qemu_capabilities.c | 8 +++---
src/qemu/qemu_cgroup.c | 4 +--
src/qemu/qemu_conf.c | 6 ++--
src/qemu/qemu_domain.c | 14 +-
src/qemu/qemu_firmware.c | 10 +++
src/qemu/qemu_migration_params.c | 4 +--
src/qemu/qemu_monitor.c | 48
src/qemu/qemu_monitor_json.c | 22 +++
src/qemu/qemu_process.c | 24
src/qemu/qemu_saveimage.c| 6 ++--
src/qemu/qemu_slirp.c| 2 +-
src/qemu/qemu_vhost_user.c | 8 +++---
13 files changed, 96 insertions(+), 96 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0b8ca2a3f5..3d88e701b2 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -77,17 +77,17 @@
qemuBlockNodeNameBackingChainDataFree(qemuBlockNodeNameBackingChainDataPtr data)
if (!data)
return;
-VIR_FREE(data->nodeformat);
-VIR_FREE(data->nodestorage);
+g_free(data->nodeformat);
+g_free(data->nodestorage);
-VIR_FREE(data->qemufilename);
+g_free(data->qemufilename);
-VIR_FREE(data->drvformat);
-VIR_FREE(data->drvstorage);
+g_free(data->drvformat);
+g_free(data->drvstorage);
qemuBlockNodeNameBackingChainDataFree(data->backing);
-VIR_FREE(data);
+g_free(data);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNodeNameBackingChainData,
@@ -1635,16 +1635,16 @@
qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data)
virJSONValueFree(data->encryptsecretProps);
virJSONValueFree(data->tlsProps);
virJSONValueFree(data->tlsKeySecretProps);
-VIR_FREE(data->tlsAlias);
-VIR_FREE(data->tlsKeySecretAlias);
-VIR_FREE(data->authsecretAlias);
-VIR_FREE(data->encryptsecretAlias);
-VIR_FREE(data->httpcookiesecretAlias);
-VIR_FREE(data->driveCmd);
-VIR_FREE(data->driveAlias);
-VIR_FREE(data->chardevAlias);
-VIR_FREE(data->chardevCmd);
-VIR_FREE(data);
+g_free(data->tlsAlias);
+g_free(data->tlsKeySecretAlias);
+g_free(data->authsecretAlias);
+g_free(data->encryptsecretAlias);
+g_free(data->httpcookiesecretAlias);
+g_free(data->driveCmd);
+g_free(data->driveAlias);
+g_free(data->chardevAlias);
+g_free(data->chardevCmd);
+g_free(data);
}
@@ -1960,8 +1960,8 @@
qemuBlockStorageSourceChainDataFree(qemuBlockStorageSourceChainDataPtr data)
for (i = 0; i < data->nsrcdata; i++)
qemuBlockStorageSourceAttachDataFree(data->srcdata[i]);
-VIR_FREE(data->srcdata);
-VIR_FREE(data);
+g_free(data->srcdata);
+g_free(data);
}
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 109a7ed9de..c57864f602 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4112,10 +4112,10 @@ virQEMUCapsCachePrivFree(void *privData)
{
virQEMUCapsCachePrivPtr priv = privData;
-VIR_FREE(priv->libDir);
-VIR_FREE(priv->kernelVersion);
-VIR_FREE(priv->hostCPUSignature);
-VIR_FREE(priv);
+g_free(priv->libDir);
+g_free(priv->kernelVersion);
+g_free(priv->hostCPUSignature);
+g_free(priv);
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index cb8112ea90..39c83d1478 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1215,8 +1215,8 @@
qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data)
return;
virCgroupFree(data->emulatorCgroup);
-VIR_FREE(data->emulatorMemMask);
-VIR_FREE(data);
+g_free(data->emulatorMemMask);
+g_free(data);
}
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 151bea01eb..eae94bb5a2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1579,9 +1579,9 @@ qemuSharedDeviceEntryFree(void *payload)
return;
for (i = 0; i < entry->ref; i++)
-VIR_FREE(entry->domains[i]);
-VIR_FREE(entry->domains);
-VIR_FREE(entry);
+g_free(entry->domains[i]);
+g_free(entry->domains);
+g_free(entry);
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a970de619e..6ab8a94b73 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1803,8 +1803,8 @@ qemuDomainObjPrivateFree(void *data)
virObjectUnref(priv->monConfig);
qemuDomainObjClearJob(>job);
-VIR_FREE(priv->lockState);
-VIR_FREE(priv->origname);
+g_free(priv->lockState);
+g_free(priv->origname);
virChrdevFree(priv->devs);
@@ -1817,7 +1817,7 @@ qemuDomainObjPrivateFree(void *data)
VIR_ERROR(_("Unexpected QEMU agent still active during domain
deletion"));
qemuAgentClose(p
[libvirt PATCH v2 02/24] util: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/util/virarptable.c | 8 +--
src/util/virauthconfig.c| 4 +-
src/util/virbitmap.c| 4 +-
src/util/vircgroup.c| 12 ++---
src/util/vircommand.c | 24 -
src/util/virconf.c | 10 ++--
src/util/virdnsmasq.c | 16 +++---
src/util/virebtables.c | 4 +-
src/util/virfdstream.c | 10 ++--
src/util/virfile.c | 4 +-
src/util/virfirewall.c | 16 +++---
src/util/virfirmware.c | 8 +--
src/util/virjson.c | 12 ++---
src/util/virlockspace.c | 12 ++---
src/util/virlog.c | 12 ++---
src/util/virmacaddr.c | 2 +-
src/util/virmdev.c | 16 +++---
src/util/virnetdev.c| 12 ++---
src/util/virnetdevbandwidth.c | 6 +--
src/util/virnetdevip.c | 6 +--
src/util/virnetdevmacvlan.c | 8 +--
src/util/virnetdevvlan.c| 2 +-
src/util/virnvme.c | 2 +-
src/util/virobject.c| 2 +-
src/util/virpci.c | 18 +++
src/util/virperf.c | 2 +-
src/util/virportallocator.c | 4 +-
src/util/virresctrl.c | 6 +--
src/util/virrotatingfile.c | 14 ++---
src/util/virscsi.c | 16 +++---
src/util/virscsivhost.c | 10 ++--
src/util/virseclabel.c | 16 +++---
src/util/virsocketaddr.c| 2 +-
src/util/virsysinfo.c | 96 -
src/util/virsystemd.c | 6 +--
src/util/virthreadpool.c| 6 +--
src/util/virtypedparam-public.c | 2 +-
src/util/virtypedparam.c| 8 +--
src/util/viruri.c | 20 +++
src/util/virusb.c | 8 +--
src/util/virxml.c | 4 +-
41 files changed, 225 insertions(+), 225 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index dac3486470..dfe3ebee5b 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -172,9 +172,9 @@ virArpTableFree(virArpTablePtr table)
return;
for (i = 0; i < table->n; i++) {
-VIR_FREE(table->t[i].ipaddr);
-VIR_FREE(table->t[i].mac);
+g_free(table->t[i].ipaddr);
+g_free(table->t[i].mac);
}
-VIR_FREE(table->t);
-VIR_FREE(table);
+g_free(table->t);
+g_free(table);
}
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 6b5487c4b0..0fc1b5c589 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -88,8 +88,8 @@ void virAuthConfigFree(virAuthConfigPtr auth)
return;
g_key_file_free(auth->keyfile);
-VIR_FREE(auth->path);
-VIR_FREE(auth);
+g_free(auth->path);
+g_free(auth);
}
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 4fde6f4fd2..b2569b0b03 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -94,8 +94,8 @@ void
virBitmapFree(virBitmapPtr bitmap)
{
if (bitmap) {
-VIR_FREE(bitmap->map);
-VIR_FREE(bitmap);
+g_free(bitmap->map);
+g_free(bitmap);
}
}
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 15071d8b1b..b11bbc5f9e 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3580,15 +3580,15 @@ virCgroupFree(virCgroupPtr group)
return;
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
-VIR_FREE(group->legacy[i].mountPoint);
-VIR_FREE(group->legacy[i].linkPoint);
-VIR_FREE(group->legacy[i].placement);
+g_free(group->legacy[i].mountPoint);
+g_free(group->legacy[i].linkPoint);
+g_free(group->legacy[i].placement);
}
-VIR_FREE(group->unified.mountPoint);
-VIR_FREE(group->unified.placement);
+g_free(group->unified.mountPoint);
+g_free(group->unified.placement);
-VIR_FREE(group);
+g_free(group);
}
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index b1a26f68aa..87a60be201 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -3028,25 +3028,25 @@ virCommandFree(virCommandPtr cmd)
VIR_FORCE_CLOSE(cmd->passfd[i].fd);
}
cmd->npassfd = 0;
-VIR_FREE(cmd->passfd);
+g_free(cmd->passfd);
if (cmd->asyncioThread) {
virThreadJoin(cmd->asyncioThread);
-VIR_FREE(cmd->asyncioThread);
+g_free(cmd->asyncioThread);
}
-VIR_FREE(cmd->inbuf);
+g_free(cmd->inbuf);
VIR_FORCE_CLOSE(cmd->outfd);
VIR_FORCE_CLOSE(cmd->errfd);
for (i = 0; i < cmd->nargs; i++)
-VIR_FREE(cmd->args[i]);
-VIR_FREE(cmd->args);
+g_free(cmd->args[i]);
+g_free(cmd->args);
for (i = 0; i < cmd->nenv; i++)
-VIR_FREE(cmd->env[i]);
-VIR_FREE(cmd->env);
+g_free(cmd->env[i]);
+g_free(cmd->env);
-VIR
[libvirt PATCH v2 04/24] libxl: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/libxl/libxl_domain.c| 2 +-
src/libxl/libxl_driver.c| 2 +-
src/libxl/libxl_migration.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 380f7e0b56..3bcd369d12 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -239,7 +239,7 @@ libxlDomainObjPrivateFree(void *data)
{
libxlDomainObjPrivatePtr priv = data;
-VIR_FREE(priv->lockState);
+g_free(priv->lockState);
virObjectUnref(priv);
}
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index f480f8067e..2e002b2930 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -127,7 +127,7 @@ libxlDomainManagedSaveLoad(virDomainObjPtr vm,
static void
libxlOSEventHookInfoFree(void *obj)
{
-VIR_FREE(obj);
+g_free(obj);
}
static void
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 29256d..72b6cb32cb 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -82,9 +82,9 @@ libxlMigrationCookieFree(libxlMigrationCookiePtr mig)
if (!mig)
return;
-VIR_FREE(mig->srcHostname);
-VIR_FREE(mig->name);
-VIR_FREE(mig);
+g_free(mig->srcHostname);
+g_free(mig->name);
+g_free(mig);
}
static libxlMigrationCookiePtr
--
2.29.2
[libvirt PATCH v2 00/24] eliminate VIR_FREE in all *Free() functions
When I sent a patch to convert (what I thought was) all of the VIR_FREEs in any *Free() function in the conf directory to g_free (with the justification that it was trivial to verify that the pointers weren't being referenced after they had been freed), there were two responses that stuck in my mind: 1) Daniel said that this was a reasonable class of uses to change and a reasonable way to arrive at the desired end result. 2) Peter justifiably expressing concern over the amount of churn in the code, and also said that he'd rather not have things "halfway" for an indeterminate time. The combination of those two comments (ignoring the part about "concern for churn" :-) let me to sit down last night and make this patch series that eliminates all uses of VIR_FREE from any function whose name ends in "Free" (or eliminate/rename the function, just for completeness' sake) In the vast majority of cases, this was done by replacing the VIR_FREEs in the functions with g_free (while verifying that everything being g_freed was actually something pointed to by the parent object that was sent to the *Free() function, and that the parent object itself was subsequently freed). But there were a a couple of cases where this wasn't quite the right thing to do: * in patch 20, two functions ending with Free() don't actually free the pointer they're sent. They instead behave like a *Clear() function, VIR_FREEing their components. Since we can see that these functions aren't actually be used as *Clear() functions (and never will be used that way), but don't want to promote confusion by leaving them misnamed, the patch renames them to *FreeContent() (more accurate name) and changes their VIR_FREEs to g_frees (it takes more than just looking at one function, but it's very easy to verify that the pointers are never referenced after they're freed). * In Patch 21-23, several *Free() functions were actually being passed a full copy of an object, and then VIR_FREEing all the pointers in the copied object. So the name was misleading, the function was inefficiently copying entire objects, and was unnecessarily NULLing the pointers that were then tossed out when the function returned. So I renamed the functions (21), changed the calling sequences to use pointers instead of copied objects (22), and (of course) changed the VIR_FREEs into g_free (23). * in Patch 24, a *Free() function actually appears to be used as a Clear() function. So I left the VIR_FREEs in place, but renamed the function to *Clear(). However, its one use as a *Clear() is when being called from a "Reset" function that (as far as I can see) is never used. If we could get rid of that Reset function, I could just fold the *Clear() function into its one remaining caller, and eliminate the VIR_FREEs. If someone knows more about the function virNetSSHSessionAuthReset() (which is in a section of the file labeled "Public API") please enlighten me. ** The last time I ran wc over the diffs of these patches, it showed they are eliminating 542 uses of VIR_FREE, which is > 13% of the 4k remaining. There is a companion series that eliminates VIR_FREE from all *Dispose() functions, but I'm sending it separately since the two series are completely independent, and I didn't want to scare anyone off. (Really, most of these can be very mechanically reviewed - just verify that all the items being g_freed are subordinates of the main argument to the function, and that that object is freed before return) Laine Stump (24): conf: replace remaining straggler VIR_FREE with g_free in vir*Free() util: replace VIR_FREE with g_free in all vir*Free() functions bhyve: replace VIR_FREE with g_free in all vir*Free() functions libxl: replace VIR_FREE with g_free in all vir*Free() functions qemu: replace VIR_FREE with g_free in all vir*Free() functions test_driver: replace VIR_FREE with g_free in all vir*Free() functions vbox: replace VIR_FREE with g_free in all vir*Free() functions vmx: replace VIR_FREE with g_free in all vir*Free() functions vz: replace VIR_FREE with g_free in all vir*Free() functions admin: replace VIR_FREE with g_free in all vir*Free() functions locking: replace VIR_FREE with g_free in all vir*Free() functions logging: replace VIR_FREE with g_free in all vir*Free() functions remote: replace VIR_FREE with g_free in all vir*Free() functions rpc: replace VIR_FREE with g_free in all vir*Free() functions security: replace VIR_FREE with g_free in all vir*Free() functions tools: replace VIR_FREE with g_free in all vir*Free() functions tests: replace VIR_FREE with g_free in all vir*Free() functions storage: replace VIR_FREE with g_free in all vir*Free() functions libvirtd: replace straggler VIR_FREE with g_free in all vir*Free() functions util: rename two *Free() functions while c
[libvirt PATCH v2 01/24] conf: replace remaining straggler VIR_FREE with g_free in vir*Free()
I missed a few in commit f9f81f1c
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 6 +++---
src/conf/numa_conf.c | 10 +-
src/conf/storage_encryption_conf.c | 2 +-
src/conf/virchrdev.c | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 12ebdb37f4..3f71297283 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26528,7 +26528,7 @@ virDomainRNGDefFree(virDomainRNGDefPtr def)
switch ((virDomainRNGBackend) def->backend) {
case VIR_DOMAIN_RNG_BACKEND_RANDOM:
-VIR_FREE(def->source.file);
+g_free(def->source.file);
break;
case VIR_DOMAIN_RNG_BACKEND_EGD:
virObjectUnref(def->source.chardev);
@@ -26539,8 +26539,8 @@ virDomainRNGDefFree(virDomainRNGDefPtr def)
}
virDomainDeviceInfoClear(>info);
-VIR_FREE(def->virtio);
-VIR_FREE(def);
+g_free(def->virtio);
+g_free(def);
}
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index f8a7a01ac9..338d779e90 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -416,15 +416,15 @@ virDomainNumaFree(virDomainNumaPtr numa)
virBitmapFree(numa->mem_nodes[i].nodeset);
if (numa->mem_nodes[i].ndistances > 0)
-VIR_FREE(numa->mem_nodes[i].distances);
+g_free(numa->mem_nodes[i].distances);
-VIR_FREE(numa->mem_nodes[i].caches);
+g_free(numa->mem_nodes[i].caches);
}
-VIR_FREE(numa->mem_nodes);
+g_free(numa->mem_nodes);
-VIR_FREE(numa->interconnects);
+g_free(numa->interconnects);
-VIR_FREE(numa);
+g_free(numa);
}
/**
diff --git a/src/conf/storage_encryption_conf.c
b/src/conf/storage_encryption_conf.c
index 7fd0df70a9..e2ae7f7b51 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -64,7 +64,7 @@ virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr
secret)
if (!secret)
return;
virSecretLookupDefClear(>seclookupdef);
-VIR_FREE(secret);
+g_free(secret);
}
void
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index d356a1babf..e5257650d4 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -223,8 +223,8 @@ static void virChrdevFDStreamCloseCbFree(void *opaque)
{
virChrdevStreamInfoPtr priv = opaque;
-VIR_FREE(priv->path);
-VIR_FREE(priv);
+g_free(priv->path);
+g_free(priv);
}
/**
@@ -304,7 +304,7 @@ void virChrdevFree(virChrdevsPtr devs)
virMutexUnlock(>lock);
virMutexDestroy(>lock);
-VIR_FREE(devs);
+g_free(devs);
}
/**
--
2.29.2
SELF NACK (Re: [libvirt PATCH 00/21] eliminate VIR_FREE in all *Free() functions)
Sigh. Self-NACK to this entire series. I accidentally took the starting commit ID for the starting patch in the git send-email command from a stale instance of gitk that was sitting on my desktop, so these are *not* the final versions or ordering of the patches :-( I'm now sending the correct versions as "PATCH v2". On 2/3/21 10:28 PM, Laine Stump wrote: When I sent a patch to convert (what I thought was) all of the VIR_FREEs in any *Free() function in the conf directory to g_free (with the justification that it was trivial to verify that the pointers weren't being referenced after they had been freed), there were two responses that stuck in my mind: 1) Daniel said that this was a reasonable class of uses to change and a reasonable way to arrive at the desired end result. 2) Peter justifiably expressing concern over the amount of churn in the code, and also said that he'd rather not have things "halfway" for an indeterminate time. The combination of those two comments (ignoring the part about "concern for churn" :-) let me to sit down last night and make this patch series that eliminates all uses of VIR_FREE from any function whose name ends in "Free" (or eliminate/rename the function, just for completeness' sake) In the vast majority of cases, this was done by replacing the VIR_FREEs in the functions with g_free (while verifying that everything being g_freed was actually something pointed to by the parent object that was sent to the *Free() function, and that the parent object itself was subsequently freed). But there were a a couple of cases where this wasn't quite the right thing to do: * in patch 20, two functions ending with Free() don't actually free the pointer they're sent. They instead behave like a *Clear() function, VIR_FREEing their components. Since we can see that these functions aren't actually be used as *Clear() functions (and never will be used that way), but don't want to promote confusion by leaving them misnamed, the patch renames them to *FreeContent() (more accurate name) and changes their VIR_FREEs to g_frees (it takes more than just looking at one function, but it's very easy to verify that the pointers are never referenced after they're freed). * In Patch 21, a *Free() function actually appears to be used as a *Clear() function, I left the VIR_FREEs in place, but renamed the *function to *Clear(). However, its one use as a *Clear() is when *being called from a "Reset" function that (as far as I can see) *is never used. If we could get rid of that Reset function, I *could just fold the *Clear() function into its one remaining *caller, and eliminate the VIR_FREEs. There is a companion series that eliminates VIR_FREE from all *Dispose() functions, but I'm sending it separately since the two series are completely independent. The last time I ran wc over the diffs of these patches, it showed they are eliminating 436 uses of VIR_FREE, which is > 10% of the 4k remaining. Laine Stump (21): conf: replace remaining straggler VIR_FREE with g_free in vir*Free() util: replace VIR_FREE with g_free in all vir*Free() functions bhyve: replace VIR_FREE with g_free in all vir*Free() functions libxl: replace VIR_FREE with g_free in all vir*Free() functions qemu: replace VIR_FREE with g_free in all vir*Free() functions test_driver: replace VIR_FREE with g_free in all vir*Free() functions vbox: replace VIR_FREE with g_free in all vir*Free() functions vmx: replace VIR_FREE with g_free in all vir*Free() functions vz: replace VIR_FREE with g_free in all vir*Free() functions admin: replace VIR_FREE with g_free in all vir*Free() functions locking: replace VIR_FREE with g_free in all vir*Free() functions logging: replace VIR_FREE with g_free in all vir*Free() functions remote: replace VIR_FREE with g_free in all vir*Free() functions rpc: replace VIR_FREE with g_free in all vir*Free() functions security: replace VIR_FREE with g_free in all vir*Free() functions tools: replace VIR_FREE with g_free in all vir*Free() functions tests: replace VIR_FREE with g_free in all vir*Free() functions storage: replace VIR_FREE with g_free in all vir*Free() functions libvirtd: replace straggler VIR_FREE with g_free in all vir*Free() functions util: rename two *Free() functions while changing VIR_FREE to g_free rpc: rename virNetSessionAuthMethodsFree to virNetSessionAuthMethodsClear src/admin/admin_server_dispatch.c | 2 +- src/bhyve/bhyve_conf.c | 6 +- src/bhyve/bhyve_domain.c| 2 +- src/conf/domain_conf.c | 6 +- src/conf/numa_conf.c| 10 +-- src/conf/storage_encryption_conf.c | 2 +- src/conf/virchrdev.c| 6 +- src/libvirt-domain.c| 18 +++--- src/libvirt-network.c | 14 ++--- sr
[libvirt PATCH 11/21] locking: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/locking/lock_daemon.c | 6 +++---
src/locking/lock_daemon_config.c | 6 +++---
src/locking/lock_driver_lockd.c | 10 +-
src/locking/lock_driver_sanlock.c | 6 +++---
src/locking/lock_manager.c| 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 851e9fc6f0..94fe374df6 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -92,7 +92,7 @@ virLockDaemonFree(virLockDaemonPtr lockd)
virHashFree(lockd->lockspaces);
virLockSpaceFree(lockd->defaultLockspace);
-VIR_FREE(lockd);
+g_free(lockd);
}
static inline void
@@ -435,8 +435,8 @@ virLockDaemonClientFree(void *opaque)
}
g_mutex_clear(>lock);
-VIR_FREE(priv->ownerName);
-VIR_FREE(priv);
+g_free(priv->ownerName);
+g_free(priv);
}
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index 6346703715..f693911997 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -71,10 +71,10 @@ virLockDaemonConfigFree(virLockDaemonConfigPtr data)
if (!data)
return;
-VIR_FREE(data->log_filters);
-VIR_FREE(data->log_outputs);
+g_free(data->log_filters);
+g_free(data->log_outputs);
-VIR_FREE(data);
+g_free(data);
}
static int
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 5ebefa48d0..a1deb55788 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -369,13 +369,13 @@
virLockManagerLockDaemonPrivateFree(virLockManagerLockDaemonPrivatePtr priv)
return;
for (i = 0; i < priv->nresources; i++) {
-VIR_FREE(priv->resources[i].lockspace);
-VIR_FREE(priv->resources[i].name);
+g_free(priv->resources[i].lockspace);
+g_free(priv->resources[i].name);
}
-VIR_FREE(priv->resources);
+g_free(priv->resources);
-VIR_FREE(priv->name);
-VIR_FREE(priv);
+g_free(priv->name);
+g_free(priv);
}
static void virLockManagerLockDaemonFree(virLockManagerPtr lock)
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index 7319f56819..aaffe30e6f 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -513,10 +513,10 @@ static void virLockManagerSanlockFree(virLockManagerPtr
lock)
if (!priv)
return;
-VIR_FREE(priv->vm_name);
+g_free(priv->vm_name);
for (i = 0; i < priv->res_count; i++)
-VIR_FREE(priv->res_args[i]);
-VIR_FREE(priv);
+g_free(priv->res_args[i]);
+g_free(priv);
lock->privateData = NULL;
}
diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
index d83a192bf6..db724eb30f 100644
--- a/src/locking/lock_manager.c
+++ b/src/locking/lock_manager.c
@@ -387,7 +387,7 @@ int virLockManagerFree(virLockManagerPtr lock)
lock->driver->drvFree(lock);
-VIR_FREE(lock);
+g_free(lock);
return 0;
}
--
2.29.2
[libvirt PATCH 08/21] vmx: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/vmx/vmx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 56318fc8b2..db535ba260 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -650,10 +650,10 @@ virVMXDomainDefNamespaceFree(void *nsdata)
struct virVMXDomainDefNamespaceData *data = nsdata;
if (data) {
-VIR_FREE(data->datacenterPath);
-VIR_FREE(data->moref);
+g_free(data->datacenterPath);
+g_free(data->moref);
}
-VIR_FREE(data);
+g_free(data);
}
static int
--
2.29.2
[libvirt PATCH 18/21] storage: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/storage/storage_backend_fs.c | 6 +++---
src/storage/storage_backend_rbd.c | 10 +-
src/storage/storage_backend_scsi.c | 4 ++--
src/storage/storage_driver.c | 4 ++--
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index b98d06c644..48924167eb 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -551,10 +551,10 @@ virStoragePoolDefFSNamespaceFree(void *nsdata)
return;
for (i = 0; i < cmdopts->noptions; i++)
-VIR_FREE(cmdopts->options[i]);
-VIR_FREE(cmdopts->options);
+g_free(cmdopts->options[i]);
+g_free(cmdopts->options);
-VIR_FREE(cmdopts);
+g_free(cmdopts);
}
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 007c53f7ac..2804759557 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -68,13 +68,13 @@ virStoragePoolDefRBDNamespaceFree(void *nsdata)
return;
for (i = 0; i < cmdopts->noptions; i++) {
-VIR_FREE(cmdopts->names[i]);
-VIR_FREE(cmdopts->values[i]);
+g_free(cmdopts->names[i]);
+g_free(cmdopts->values[i]);
}
-VIR_FREE(cmdopts->names);
-VIR_FREE(cmdopts->values);
+g_free(cmdopts->names);
+g_free(cmdopts->values);
-VIR_FREE(cmdopts);
+g_free(cmdopts);
}
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 69a01d1a24..9474c60c72 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -95,8 +95,8 @@ virStoragePoolFCRefreshDataFree(void *opaque)
{
virStoragePoolFCRefreshInfoPtr cbdata = opaque;
-VIR_FREE(cbdata->fchost_name);
-VIR_FREE(cbdata);
+g_free(cbdata->fchost_name);
+g_free(cbdata);
}
/**
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 16bc53aa46..9c5be071df 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2268,8 +2268,8 @@ virStorageVolPoolRefreshDataFree(void *opaque)
{
virStorageVolStreamInfoPtr cbdata = opaque;
-VIR_FREE(cbdata->pool_name);
-VIR_FREE(cbdata);
+g_free(cbdata->pool_name);
+g_free(cbdata);
}
static int
--
2.29.2
[libvirt PATCH 20/21] util: rename two *Free() functions while changing VIR_FREE to g_free
dhcpHostFree() and addnHostFree() don't follow the normal pattern of
*Free functions in the rest of libvirt code - they are actually more
similar to the *Dispose() functions, in that they free all subordinate
objects, but not the object pointed to by the argument
itself. However, the arguments aren't virObjects, so it wouldn't be
proper to name them *Dispose() either.
They *currently* behave similar to a *Clear() function, in that they
free all the subordinate objects and nullify the pointers of those
objects. HOWEVER, we don't actually need or want that behavior - the
two functions in question are only called as part of a higher level
*Free() function, and the pointers are not referenced in any way
between the time they are freed and when the parent object is freed.
So, since the current name isn't correct, nor is *Dispose(), and we
want to change the behavior in such a way that *Clear() also wouldn't
be correct, lets name the functions *FreeContent(), which is an
accurate description of what the functions do, and what we *want* them
to do.
And since it's such a small patch, we can go ahead and change that
behavior - replacing the VIR_FREEs with g_free.
Signed-off-by: Laine Stump
---
src/util/virdnsmasq.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index c21819c981..653d46bef9 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -50,20 +50,20 @@ VIR_LOG_INIT("util.dnsmasq");
#define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
static void
-dhcphostFree(dnsmasqDhcpHost *host)
+dhcphostFreeContent(dnsmasqDhcpHost *host)
{
-VIR_FREE(host->host);
+g_free(host->host);
}
static void
-addnhostFree(dnsmasqAddnHost *host)
+addnhostFreeContent(dnsmasqAddnHost *host)
{
size_t i;
for (i = 0; i < host->nhostnames; i++)
-VIR_FREE(host->hostnames[i]);
-VIR_FREE(host->hostnames);
-VIR_FREE(host->ip);
+g_free(host->hostnames[i]);
+g_free(host->hostnames);
+g_free(host->ip);
}
static void
@@ -73,7 +73,7 @@ addnhostsFree(dnsmasqAddnHostsfile *addnhostsfile)
if (addnhostsfile->hosts) {
for (i = 0; i < addnhostsfile->nhosts; i++)
-addnhostFree(>hosts[i]);
+addnhostFreeContent(>hosts[i]);
g_free(addnhostsfile->hosts);
@@ -270,7 +270,7 @@ hostsfileFree(dnsmasqHostsfile *hostsfile)
if (hostsfile->hosts) {
for (i = 0; i < hostsfile->nhosts; i++)
-dhcphostFree(>hosts[i]);
+dhcphostFreeContent(>hosts[i]);
g_free(hostsfile->hosts);
--
2.29.2
[libvirt PATCH 13/21] remote: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/remote/remote_daemon_config.c | 48 ++---
src/remote/remote_daemon_dispatch.c | 4 +--
src/remote/remote_driver.c | 2 +-
3 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/remote/remote_daemon_config.c
b/src/remote/remote_daemon_config.c
index f0cca42925..b354dc4cc0 100644
--- a/src/remote/remote_daemon_config.c
+++ b/src/remote/remote_daemon_config.c
@@ -173,53 +173,53 @@ daemonConfigFree(struct daemonConfig *data)
return;
#ifdef WITH_IP
-VIR_FREE(data->listen_addr);
-VIR_FREE(data->tls_port);
-VIR_FREE(data->tcp_port);
+g_free(data->listen_addr);
+g_free(data->tls_port);
+g_free(data->tcp_port);
#endif /* ! WITH_IP */
tmp = data->access_drivers;
while (tmp && *tmp) {
-VIR_FREE(*tmp);
+g_free(*tmp);
tmp++;
}
-VIR_FREE(data->access_drivers);
+g_free(data->access_drivers);
-VIR_FREE(data->unix_sock_admin_perms);
-VIR_FREE(data->unix_sock_ro_perms);
-VIR_FREE(data->unix_sock_rw_perms);
-VIR_FREE(data->unix_sock_group);
-VIR_FREE(data->unix_sock_dir);
+g_free(data->unix_sock_admin_perms);
+g_free(data->unix_sock_ro_perms);
+g_free(data->unix_sock_rw_perms);
+g_free(data->unix_sock_group);
+g_free(data->unix_sock_dir);
tmp = data->sasl_allowed_username_list;
while (tmp && *tmp) {
-VIR_FREE(*tmp);
+g_free(*tmp);
tmp++;
}
-VIR_FREE(data->sasl_allowed_username_list);
+g_free(data->sasl_allowed_username_list);
#ifdef WITH_IP
tmp = data->tls_allowed_dn_list;
while (tmp && *tmp) {
-VIR_FREE(*tmp);
+g_free(*tmp);
tmp++;
}
-VIR_FREE(data->tls_allowed_dn_list);
+g_free(data->tls_allowed_dn_list);
-VIR_FREE(data->tls_priority);
+g_free(data->tls_priority);
-VIR_FREE(data->key_file);
-VIR_FREE(data->ca_file);
-VIR_FREE(data->cert_file);
-VIR_FREE(data->crl_file);
+g_free(data->key_file);
+g_free(data->ca_file);
+g_free(data->cert_file);
+g_free(data->crl_file);
#endif /* ! WITH_IP */
-VIR_FREE(data->host_uuid);
-VIR_FREE(data->host_uuid_source);
-VIR_FREE(data->log_filters);
-VIR_FREE(data->log_outputs);
+g_free(data->host_uuid);
+g_free(data->host_uuid_source);
+g_free(data->log_filters);
+g_free(data->log_outputs);
-VIR_FREE(data);
+g_free(data);
}
static int
diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
index 46683aa4a7..b7ef1f4b26 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -150,7 +150,7 @@ remoteEventCallbackFree(void *opaque)
return;
virObjectUnref(callback->program);
virObjectUnref(callback->client);
-VIR_FREE(callback);
+g_free(callback);
}
@@ -1742,7 +1742,7 @@ void remoteClientFree(void *data)
if (priv->storageConn)
virConnectClose(priv->storageConn);
-VIR_FREE(priv);
+g_free(priv);
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 1b784e61c7..8d6790ccf2 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5614,7 +5614,7 @@ static void remoteStreamCallbackFree(void *opaque)
(cbdata->ff)(cbdata->opaque);
virObjectUnref(cbdata->st);
-VIR_FREE(opaque);
+g_free(opaque);
}
--
2.29.2
[libvirt PATCH 05/21] qemu: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/qemu/qemu_block.c| 36
src/qemu/qemu_capabilities.c | 8 +++---
src/qemu/qemu_cgroup.c | 4 +--
src/qemu/qemu_conf.c | 6 ++--
src/qemu/qemu_domain.c | 14 +-
src/qemu/qemu_firmware.c | 10 +++
src/qemu/qemu_migration_params.c | 4 +--
src/qemu/qemu_monitor.c | 48
src/qemu/qemu_monitor_json.c | 22 +++
src/qemu/qemu_process.c | 24
src/qemu/qemu_saveimage.c| 6 ++--
src/qemu/qemu_slirp.c| 2 +-
src/qemu/qemu_vhost_user.c | 8 +++---
13 files changed, 96 insertions(+), 96 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0b8ca2a3f5..3d88e701b2 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -77,17 +77,17 @@
qemuBlockNodeNameBackingChainDataFree(qemuBlockNodeNameBackingChainDataPtr data)
if (!data)
return;
-VIR_FREE(data->nodeformat);
-VIR_FREE(data->nodestorage);
+g_free(data->nodeformat);
+g_free(data->nodestorage);
-VIR_FREE(data->qemufilename);
+g_free(data->qemufilename);
-VIR_FREE(data->drvformat);
-VIR_FREE(data->drvstorage);
+g_free(data->drvformat);
+g_free(data->drvstorage);
qemuBlockNodeNameBackingChainDataFree(data->backing);
-VIR_FREE(data);
+g_free(data);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNodeNameBackingChainData,
@@ -1635,16 +1635,16 @@
qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data)
virJSONValueFree(data->encryptsecretProps);
virJSONValueFree(data->tlsProps);
virJSONValueFree(data->tlsKeySecretProps);
-VIR_FREE(data->tlsAlias);
-VIR_FREE(data->tlsKeySecretAlias);
-VIR_FREE(data->authsecretAlias);
-VIR_FREE(data->encryptsecretAlias);
-VIR_FREE(data->httpcookiesecretAlias);
-VIR_FREE(data->driveCmd);
-VIR_FREE(data->driveAlias);
-VIR_FREE(data->chardevAlias);
-VIR_FREE(data->chardevCmd);
-VIR_FREE(data);
+g_free(data->tlsAlias);
+g_free(data->tlsKeySecretAlias);
+g_free(data->authsecretAlias);
+g_free(data->encryptsecretAlias);
+g_free(data->httpcookiesecretAlias);
+g_free(data->driveCmd);
+g_free(data->driveAlias);
+g_free(data->chardevAlias);
+g_free(data->chardevCmd);
+g_free(data);
}
@@ -1960,8 +1960,8 @@
qemuBlockStorageSourceChainDataFree(qemuBlockStorageSourceChainDataPtr data)
for (i = 0; i < data->nsrcdata; i++)
qemuBlockStorageSourceAttachDataFree(data->srcdata[i]);
-VIR_FREE(data->srcdata);
-VIR_FREE(data);
+g_free(data->srcdata);
+g_free(data);
}
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 109a7ed9de..c57864f602 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4112,10 +4112,10 @@ virQEMUCapsCachePrivFree(void *privData)
{
virQEMUCapsCachePrivPtr priv = privData;
-VIR_FREE(priv->libDir);
-VIR_FREE(priv->kernelVersion);
-VIR_FREE(priv->hostCPUSignature);
-VIR_FREE(priv);
+g_free(priv->libDir);
+g_free(priv->kernelVersion);
+g_free(priv->hostCPUSignature);
+g_free(priv);
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index cb8112ea90..39c83d1478 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1215,8 +1215,8 @@
qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data)
return;
virCgroupFree(data->emulatorCgroup);
-VIR_FREE(data->emulatorMemMask);
-VIR_FREE(data);
+g_free(data->emulatorMemMask);
+g_free(data);
}
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 151bea01eb..eae94bb5a2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1579,9 +1579,9 @@ qemuSharedDeviceEntryFree(void *payload)
return;
for (i = 0; i < entry->ref; i++)
-VIR_FREE(entry->domains[i]);
-VIR_FREE(entry->domains);
-VIR_FREE(entry);
+g_free(entry->domains[i]);
+g_free(entry->domains);
+g_free(entry);
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a970de619e..6ab8a94b73 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1803,8 +1803,8 @@ qemuDomainObjPrivateFree(void *data)
virObjectUnref(priv->monConfig);
qemuDomainObjClearJob(>job);
-VIR_FREE(priv->lockState);
-VIR_FREE(priv->origname);
+g_free(priv->lockState);
+g_free(priv->origname);
virChrdevFree(priv->devs);
@@ -1817,7 +1817,7 @@ qemuDomainObjPrivateFree(void *data)
VIR_ERROR(_("Unexpected QEMU agent still active during domain
deletion"));
qemuAgentClose(p
[libvirt PATCH 21/21] rpc: rename virNetSessionAuthMethodsFree to virNetSessionAuthMethodsClear
This is another *Free() function that doesn't free the object it is
passed. Instead it frees and clears some parts of the object.
In this case, the function is actually called from two places, and one
of them (virNetSSHSessionAuthReset) appears to be assuming that the
pointers actually *will* be cleared. So the proper thing to do here
(?) is to rename the function to virNetSSHSesionAuthMethodsClear().
(NB: virNetSSHSessionAuthReset is seemingly never called from
anywhere. Is this one of those functions that actually *is* called by
some strange MACRO invocation? Or it is truly one of those
"written-but-never-used" functions that can be deleted? (if the latter
is the case, then I would rather move the contents of
virNetSessionAuthMethodsFree() into its only other caller,
virNetSSHSessionDispose(), so that the VIR_FREEs could be replaced
with g_free.)
Signed-off-by: Laine Stump
---
src/rpc/virnetsshsession.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c
index 4b56363fa0..5bfe311544 100644
--- a/src/rpc/virnetsshsession.c
+++ b/src/rpc/virnetsshsession.c
@@ -113,7 +113,7 @@ struct _virNetSSHSession {
};
static void
-virNetSSHSessionAuthMethodsFree(virNetSSHSessionPtr sess)
+virNetSSHSessionAuthMethodsClear(virNetSSHSessionPtr sess)
{
size_t i;
@@ -149,7 +149,7 @@ virNetSSHSessionDispose(void *obj)
libssh2_session_free(sess->session);
}
-virNetSSHSessionAuthMethodsFree(sess);
+virNetSSHSessionAuthMethodsClear(sess);
VIR_FREE(sess->channelCommand);
VIR_FREE(sess->hostname);
@@ -971,7 +971,7 @@ void
virNetSSHSessionAuthReset(virNetSSHSessionPtr sess)
{
virObjectLock(sess);
-virNetSSHSessionAuthMethodsFree(sess);
+virNetSSHSessionAuthMethodsClear(sess);
virObjectUnlock(sess);
}
--
2.29.2
[libvirt PATCH 12/21] logging: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/logging/log_daemon.c| 4 ++-- src/logging/log_daemon_config.c | 6 +++--- src/logging/log_handler.c | 6 +++--- src/logging/log_manager.c | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 6b8f3b6fe5..770f6dd273 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -89,7 +89,7 @@ virLogDaemonFree(virLogDaemonPtr logd) g_mutex_clear(>lock); virObjectUnref(logd->dmn); -VIR_FREE(logd); +g_free(logd); } @@ -314,7 +314,7 @@ virLogDaemonClientFree(void *opaque) (unsigned long long)priv->clientPid); g_mutex_clear(>lock); -VIR_FREE(priv); +g_free(priv); } diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index 5577991b13..075cf766ec 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -74,10 +74,10 @@ virLogDaemonConfigFree(virLogDaemonConfigPtr data) if (!data) return; -VIR_FREE(data->log_filters); -VIR_FREE(data->log_outputs); +g_free(data->log_filters); +g_free(data->log_outputs); -VIR_FREE(data); +g_free(data); } static int diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 95d909b44e..45a4763525 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -98,9 +98,9 @@ virLogHandlerLogFileFree(virLogHandlerLogFilePtr file) if (file->watch != -1) virEventRemoveHandle(file->watch); -VIR_FREE(file->driver); -VIR_FREE(file->domname); -VIR_FREE(file); +g_free(file->driver); +g_free(file->domname); +g_free(file); } diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c index 57be340a2d..56927c1d6d 100644 --- a/src/logging/log_manager.c +++ b/src/logging/log_manager.c @@ -137,7 +137,7 @@ virLogManagerFree(virLogManagerPtr mgr) virObjectUnref(mgr->program); virObjectUnref(mgr->client); -VIR_FREE(mgr); +g_free(mgr); } -- 2.29.2
[libvirt PATCH 06/21] test_driver: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/test/test_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 29c4c86b1d..bca1297d1d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -198,8 +198,8 @@ testDomainDefNamespaceFree(void *data)
for (i = 0; i < nsdata->num_snap_nodes; i++)
xmlFreeNode(nsdata->snap_nodes[i]);
-VIR_FREE(nsdata->snap_nodes);
-VIR_FREE(nsdata);
+g_free(nsdata->snap_nodes);
+g_free(nsdata);
}
static int
@@ -428,7 +428,7 @@ static void
testDomainObjPrivateFree(void *data)
{
testDomainObjPrivatePtr priv = data;
-VIR_FREE(priv);
+g_free(priv);
}
--
2.29.2
[libvirt PATCH 04/21] libxl: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
src/libxl/libxl_domain.c| 2 +-
src/libxl/libxl_driver.c| 2 +-
src/libxl/libxl_migration.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 380f7e0b56..3bcd369d12 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -239,7 +239,7 @@ libxlDomainObjPrivateFree(void *data)
{
libxlDomainObjPrivatePtr priv = data;
-VIR_FREE(priv->lockState);
+g_free(priv->lockState);
virObjectUnref(priv);
}
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index f480f8067e..2e002b2930 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -127,7 +127,7 @@ libxlDomainManagedSaveLoad(virDomainObjPtr vm,
static void
libxlOSEventHookInfoFree(void *obj)
{
-VIR_FREE(obj);
+g_free(obj);
}
static void
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 29256d..72b6cb32cb 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -82,9 +82,9 @@ libxlMigrationCookieFree(libxlMigrationCookiePtr mig)
if (!mig)
return;
-VIR_FREE(mig->srcHostname);
-VIR_FREE(mig->name);
-VIR_FREE(mig);
+g_free(mig->srcHostname);
+g_free(mig->name);
+g_free(mig);
}
static libxlMigrationCookiePtr
--
2.29.2
[libvirt PATCH 09/21] vz: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump --- src/vz/vz_driver.c | 8 src/vz/vz_utils.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index b60e99d4f5..0ebcb06234 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -2729,10 +2729,10 @@ vzMigrationCookieFree(vzMigrationCookiePtr mig) if (!mig) return; -VIR_FREE(mig->session_uuid); -VIR_FREE(mig->uuid); -VIR_FREE(mig->name); -VIR_FREE(mig); +g_free(mig->session_uuid); +g_free(mig->uuid); +g_free(mig->name); +g_free(mig); } static int diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 4a79b55f3e..e5106619b6 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -582,7 +582,7 @@ vzDomObjFree(void* p) PrlHandle_Free(pdom->sdkdom); PrlHandle_Free(pdom->stats); virCondDestroy(>job.cond); -VIR_FREE(pdom); +g_free(pdom); }; #define VZ_JOB_WAIT_TIME (1000 * 30) -- 2.29.2
[libvirt PATCH 17/21] tests: replace VIR_FREE with g_free in all vir*Free() functions
Signed-off-by: Laine Stump
---
tests/nodedevmdevctltest.c | 4 ++--
tests/nwfilterxml2firewalltest.c | 2 +-
tests/qemuhotplugtest.c | 12 ++--
tests/qemumonitortestutils.c | 28 ++--
tests/virnetdaemontest.c | 2 +-
tests/virnetserverclienttest.c | 2 +-
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
index dab4b487b9..1bad65549b 100644
--- a/tests/nodedevmdevctltest.c
+++ b/tests/nodedevmdevctltest.c
@@ -152,8 +152,8 @@ nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv)
virNodeDeviceObjListFree(drv->devs);
virCondDestroy(>initCond);
virMutexDestroy(>lock);
-VIR_FREE(drv->stateDir);
-VIR_FREE(drv);
+g_free(drv->stateDir);
+g_free(drv);
}
/* Add a fake root 'computer' device */
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index 0901250aaf..3e2ab0b0ba 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -174,7 +174,7 @@ virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst)
return;
virHashFree(inst->vars);
-VIR_FREE(inst);
+g_free(inst);
}
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 776fb019f3..df5c9c9059 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -383,12 +383,12 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData
*data)
if (!data)
return;
-VIR_FREE(data->file_xml_dom);
-VIR_FREE(data->file_xml_res_live);
-VIR_FREE(data->file_xml_res_conf);
-VIR_FREE(data->file_json_monitor);
+g_free(data->file_xml_dom);
+g_free(data->file_xml_res_live);
+g_free(data->file_xml_res_conf);
+g_free(data->file_json_monitor);
-VIR_FREE(data->xml_dom);
+g_free(data->xml_dom);
if (data->vm) {
priv = data->vm->privateData;
@@ -402,7 +402,7 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData
*data)
virObjectLock(mon);
qemuMonitorTestFree(data->mon);
}
-VIR_FREE(data);
+g_free(data);
}
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c4f7082655..ae3fcf9311 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -99,7 +99,7 @@ qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)
if (item->freecb)
(item->freecb)(item->opaque);
-VIR_FREE(item);
+g_free(item);
}
@@ -423,8 +423,8 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
if (timer != -1)
virEventRemoveTimeout(timer);
-VIR_FREE(test->incoming);
-VIR_FREE(test->outgoing);
+g_free(test->incoming);
+g_free(test->outgoing);
for (i = 0; i < test->nitems; i++) {
if (!test->allowUnusedCommands) {
@@ -435,12 +435,12 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
qemuMonitorTestItemFree(test->items[i]);
}
-VIR_FREE(test->items);
+g_free(test->items);
if (test->tmpdir && rmdir(test->tmpdir) < 0)
VIR_WARN("Failed to remove tempdir: %s", g_strerror(errno));
-VIR_FREE(test->tmpdir);
+g_free(test->tmpdir);
if (!test->allowUnusedCommands &&
test->nitems != 0) {
@@ -448,7 +448,7 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
}
virMutexDestroy(>lock);
-VIR_FREE(test);
+g_free(test);
}
@@ -518,16 +518,16 @@ qemuMonitorTestHandlerDataFree(void *opaque)
return;
for (i = 0; i < data->nargs; i++) {
-VIR_FREE(data->args[i].argname);
-VIR_FREE(data->args[i].argval);
+g_free(data->args[i].argname);
+g_free(data->args[i].argval);
}
-VIR_FREE(data->command_name);
-VIR_FREE(data->cmderr);
-VIR_FREE(data->response);
-VIR_FREE(data->args);
-VIR_FREE(data->expectArgs);
-VIR_FREE(data);
+g_free(data->command_name);
+g_free(data->cmderr);
+g_free(data->response);
+g_free(data->args);
+g_free(data->expectArgs);
+g_free(data);
}
diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c
index a20f351f93..fb40fe9b80 100644
--- a/tests/virnetdaemontest.c
+++ b/tests/virnetdaemontest.c
@@ -76,7 +76,7 @@ testClientNewPostExec(virNetServerClientPtr client,
static void
testClientFree(void *opaque)
{
-VIR_FREE(opaque);
+g_free(opaque);
}
diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c
index a4341c1641..959100ec66 100644
--- a/tests/virnetserverclienttest.c
+++ b/tests/virnetserverclienttest.c
@@ -41,7 +41,7 @@ testClientNew(virNetServerClientPtr client G_GNUC_UNUSED,
static void
testClientFree(void *opaque)
{
-VIR_FREE(opaque);
+g_free(opaque);
}
static int testIdentity(const void *opaque G_GNUC_UNUSED)
--
2.29.2
