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@gmail.com> --- 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@redhat.com> 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