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 <danielhb413@xxxxxxxxx>
---
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 <laine@xxxxxxxxxx>
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, &devAddr) < 0)
- goto cleanup;
+ return -1;
pci = virPCIDeviceNew(&devAddr);
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
