[PATCH 00/10] virclosecallbacks: Cleanups (part 1)
This series simplifies some aspects of close callbacks in preparation of further refactors. Peter Krempa (10): virclosecallbacks.h: Reformat header to contemporary style virObjectLockGuard: Require that returned value is used qemuMigrationSrcBegin: Automatically free 'xml' variable on error bhyve: Store 'driver' in VM private data lxc: Store 'driver' in VM private data qemuMigrationSrcCleanup: Use 'driver' from VM private data qemuProcessAutoDestroy: Use 'driver' from VM private data lxcProcessAutoDestroy: Use 'driver' from VM private data bhyveProcessAutoDestroy: Use 'driver' from VM private data virclosecallbacks: Don't pass opqaue pointer to callback invocation src/bhyve/bhyve_domain.c | 8 ++++++-- src/bhyve/bhyve_domain.h | 2 ++ src/bhyve/bhyve_driver.c | 2 +- src/bhyve/bhyve_process.c | 6 +++--- src/hypervisor/virclosecallbacks.c | 5 ++--- src/hypervisor/virclosecallbacks.h | 29 +++++++++++++++++------------ src/lxc/lxc_domain.c | 4 +++- src/lxc/lxc_domain.h | 1 + src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 8 +++----- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 13 +++++-------- src/qemu/qemu_process.c | 5 ++--- src/util/virobject.h | 2 +- 14 files changed, 48 insertions(+), 41 deletions(-) -- 2.35.3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/hypervisor/virclosecallbacks.h | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/hypervisor/virclosecallbacks.h b/src/hypervisor/virclosecallbacks.h index 97be25b781..138e0eb689 100644 --- a/src/hypervisor/virclosecallbacks.h +++ b/src/hypervisor/virclosecallbacks.h @@ -27,14 +27,20 @@ typedef struct _virCloseCallbacks virCloseCallbacks; typedef void (*virCloseCallback)(virDomainObj *vm, virConnectPtr conn, void *opaque); -virCloseCallbacks *virCloseCallbacksNew(void); -int virCloseCallbacksSet(virCloseCallbacks *closeCallbacks, - virDomainObj *vm, - virConnectPtr conn, - virCloseCallback cb); -int virCloseCallbacksUnset(virCloseCallbacks *closeCallbacks, - virDomainObj *vm, - virCloseCallback cb); + +virCloseCallbacks * +virCloseCallbacksNew(void); + +int +virCloseCallbacksSet(virCloseCallbacks *closeCallbacks, + virDomainObj *vm, + virConnectPtr conn, + virCloseCallback cb); +int +virCloseCallbacksUnset(virCloseCallbacks *closeCallbacks, + virDomainObj *vm, + virCloseCallback cb); + virCloseCallback virCloseCallbacksGet(virCloseCallbacks *closeCallbacks, virDomainObj *vm, @@ -42,6 +48,7 @@ virCloseCallbacksGet(virCloseCallbacks *closeCallbacks, virConnectPtr virCloseCallbacksGetConn(virCloseCallbacks *closeCallbacks, virDomainObj *vm); + void virCloseCallbacksRun(virCloseCallbacks *closeCallbacks, virConnectPtr conn, -- 2.35.3
The returned value is used to unlock the object, so all callers must necessarily make use of the returned value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virobject.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virobject.h b/src/util/virobject.h index a1e16aee77..4b941ac215 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -120,7 +120,7 @@ virObjectRWLockableNew(virClass *klass) virLockGuard virObjectLockGuard(void *lockableobj) - ATTRIBUTE_NONNULL(1); + ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; void virObjectLock(void *lockableobj) -- 2.35.3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 25af291dc6..6b3815ac58 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2487,7 +2487,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, { virQEMUDriver *driver = conn->privateData; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - char *xml = NULL; + g_autofree char *xml = NULL; virDomainAsyncJob asyncJob; if (cfg->migrateTLSForce && @@ -2532,10 +2532,8 @@ qemuMigrationSrcBegin(virConnectPtr conn, * place. */ if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, - qemuMigrationSrcCleanup) < 0) { - VIR_FREE(xml); + qemuMigrationSrcCleanup) < 0) goto endjob; - } qemuMigrationJobContinue(vm); } else { goto endjob; @@ -2543,7 +2541,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, cleanup: virDomainObjEndAPI(&vm); - return xml; + return g_steal_pointer(&xml); endjob: if (flags & VIR_MIGRATE_CHANGE_PROTECTION) -- 2.35.3
Similarly to the qemu driver if we store the immutable driver pointer in the VM private data struct we don't have to questionably pass it through opaque pointers to callbacks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_domain.c | 8 ++++++-- src/bhyve/bhyve_domain.h | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index b526235a4e..69555a3efc 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -34,9 +34,13 @@ VIR_LOG_INIT("bhyve.bhyve_domain"); static void * -bhyveDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED) +bhyveDomainObjPrivateAlloc(void *opaque) { - return g_new0(bhyveDomainObjPrivate, 1); + bhyveDomainObjPrivate *priv = g_new0(bhyveDomainObjPrivate, 1); + + priv->driver = opaque; + + return priv; } static void diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h index 89a4a3c7cb..5a539bc4c0 100644 --- a/src/bhyve/bhyve_domain.h +++ b/src/bhyve/bhyve_domain.h @@ -27,6 +27,8 @@ typedef struct _bhyveDomainObjPrivate bhyveDomainObjPrivate; struct _bhyveDomainObjPrivate { + struct _bhyveConn *driver; + virDomainPCIAddressSet *pciaddrs; bool persistentAddrs; -- 2.35.3
Similarly to the qemu driver if we store the immutable driver pointer in the VM private data struct we don't have to questionably pass it through opaque pointers to callbacks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_domain.c | 4 +++- src/lxc/lxc_domain.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index fae56b35fb..5064f942b9 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -143,7 +143,7 @@ virLXCDomainObjEndJob(virLXCDriver *driver G_GNUC_UNUSED, static void * -virLXCDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED) +virLXCDomainObjPrivateAlloc(void *opaque) { virLXCDomainObjPrivate *priv = g_new0(virLXCDomainObjPrivate, 1); @@ -152,6 +152,8 @@ virLXCDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED) return NULL; } + priv->driver = opaque; + return priv; } diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h index 1c4cb8c14a..d91dcca04b 100644 --- a/src/lxc/lxc_domain.h +++ b/src/lxc/lxc_domain.h @@ -63,6 +63,7 @@ struct virLXCDomainJobObj { typedef struct _virLXCDomainObjPrivate virLXCDomainObjPrivate; struct _virLXCDomainObjPrivate { + virLXCDriver *driver; virLXCMonitor *monitor; bool doneStopEvent; int stopReason; -- 2.35.3
Access the 'driver' struct from the private data rather than the passed opaque pointer in preparation to remove the opaque pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6b3815ac58..7de0929a36 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2164,10 +2164,10 @@ qemuMigrationDstRun(virQEMUDriver *driver, static void qemuMigrationSrcCleanup(virDomainObj *vm, virConnectPtr conn, - void *opaque) + void *opaque G_GNUC_UNUSED) { - virQEMUDriver *driver = opaque; qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; qemuDomainJobPrivate *jobPriv = priv->job.privateData; VIR_DEBUG("vm=%s, conn=%p, asyncJob=%s, phase=%s", -- 2.35.3
Access the 'driver' struct from the private data rather than the passed opaque pointer in preparation to remove the opaque pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 07e467d01e..a66ca9aa60 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8387,10 +8387,10 @@ void qemuProcessStop(virQEMUDriver *driver, static void qemuProcessAutoDestroy(virDomainObj *dom, virConnectPtr conn, - void *opaque) + void *opaque G_GNUC_UNUSED) { - virQEMUDriver *driver = opaque; qemuDomainObjPrivate *priv = dom->privateData; + virQEMUDriver *driver = priv->driver; virObjectEvent *event = NULL; unsigned int stopFlags = 0; -- 2.35.3
Access the 'driver' struct from the private data rather than the passed opaque pointer in preparation to remove the opaque pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_process.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0222e8a9b3..368adaea7e 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -63,15 +63,14 @@ VIR_LOG_INIT("lxc.lxc_process"); static void lxcProcessAutoDestroy(virDomainObj *dom, virConnectPtr conn, - void *opaque) + void *opaque G_GNUC_UNUSED) { - virLXCDriver *driver = opaque; virObjectEvent *event = NULL; - virLXCDomainObjPrivate *priv; + virLXCDomainObjPrivate *priv = dom->privateData; + virLXCDriver *driver = priv->driver; VIR_DEBUG("driver=%p dom=%s conn=%p", driver, dom->def->name, conn); - priv = dom->privateData; VIR_DEBUG("Killing domain"); virLXCProcessStop(driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED); virDomainAuditStop(dom, "destroyed"); -- 2.35.3
Access the 'driver' struct from the private data rather than the passed opaque pointer in preparation to remove the opaque pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_process.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index ee692d2ba3..40c97cb6ca 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -57,9 +57,10 @@ VIR_LOG_INIT("bhyve.bhyve_process"); static void bhyveProcessAutoDestroy(virDomainObj *vm, virConnectPtr conn G_GNUC_UNUSED, - void *opaque) + void *opaque G_GNUC_UNUSED) { - struct _bhyveConn *driver = opaque; + bhyveDomainObjPrivate *priv = vm->privateData; + struct _bhyveConn *driver = priv->driver; virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); -- 2.35.3
Remove the argument from the function prototypes and the callback handler. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_driver.c | 2 +- src/bhyve/bhyve_process.c | 3 +-- src/hypervisor/virclosecallbacks.c | 5 ++--- src/hypervisor/virclosecallbacks.h | 6 ++---- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 3 +-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 3 +-- src/qemu/qemu_process.c | 3 +-- 9 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 51973ae670..09ba52483a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -207,7 +207,7 @@ bhyveConnectClose(virConnectPtr conn) { struct _bhyveConn *privconn = conn->privateData; - virCloseCallbacksRun(privconn->closeCallbacks, conn, privconn->domains, privconn); + virCloseCallbacksRun(privconn->closeCallbacks, conn, privconn->domains); conn->privateData = NULL; return 0; diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 40c97cb6ca..18002b559b 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -56,8 +56,7 @@ VIR_LOG_INIT("bhyve.bhyve_process"); static void bhyveProcessAutoDestroy(virDomainObj *vm, - virConnectPtr conn G_GNUC_UNUSED, - void *opaque G_GNUC_UNUSED) + virConnectPtr conn G_GNUC_UNUSED) { bhyveDomainObjPrivate *priv = vm->privateData; struct _bhyveConn *driver = priv->driver; diff --git a/src/hypervisor/virclosecallbacks.c b/src/hypervisor/virclosecallbacks.c index 84a61b002a..c533e695f1 100644 --- a/src/hypervisor/virclosecallbacks.c +++ b/src/hypervisor/virclosecallbacks.c @@ -284,8 +284,7 @@ virCloseCallbacksGetForConn(virCloseCallbacks *closeCallbacks, void virCloseCallbacksRun(virCloseCallbacks *closeCallbacks, virConnectPtr conn, - virDomainObjList *domains, - void *opaque) + virDomainObjList *domains) { virCloseCallbacksList *list; size_t i; @@ -329,7 +328,7 @@ virCloseCallbacksRun(virCloseCallbacks *closeCallbacks, * * Call the callback function and end the API usage. */ virObjectUnref(vm); - list->entries[i].callback(vm, conn, opaque); + list->entries[i].callback(vm, conn); virDomainObjEndAPI(&vm); } VIR_FREE(list->entries); diff --git a/src/hypervisor/virclosecallbacks.h b/src/hypervisor/virclosecallbacks.h index 138e0eb689..5a56dea292 100644 --- a/src/hypervisor/virclosecallbacks.h +++ b/src/hypervisor/virclosecallbacks.h @@ -25,8 +25,7 @@ typedef struct _virCloseCallbacks virCloseCallbacks; typedef void (*virCloseCallback)(virDomainObj *vm, - virConnectPtr conn, - void *opaque); + virConnectPtr conn); virCloseCallbacks * virCloseCallbacksNew(void); @@ -52,5 +51,4 @@ virCloseCallbacksGetConn(virCloseCallbacks *closeCallbacks, void virCloseCallbacksRun(virCloseCallbacks *closeCallbacks, virConnectPtr conn, - virDomainObjList *domains, - void *opaque); + virDomainObjList *domains); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a42299eeea..31de677b88 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -169,7 +169,7 @@ static int lxcConnectClose(virConnectPtr conn) { virLXCDriver *driver = conn->privateData; - virCloseCallbacksRun(driver->closeCallbacks, conn, driver->domains, driver); + virCloseCallbacksRun(driver->closeCallbacks, conn, driver->domains); conn->privateData = NULL; return 0; } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 368adaea7e..9722d8e1de 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -62,8 +62,7 @@ VIR_LOG_INIT("lxc.lxc_process"); static void lxcProcessAutoDestroy(virDomainObj *dom, - virConnectPtr conn, - void *opaque G_GNUC_UNUSED) + virConnectPtr conn) { virObjectEvent *event = NULL; virLXCDomainObjPrivate *priv = dom->privateData; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 702fd0239c..1d39af1891 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1161,7 +1161,7 @@ static int qemuConnectClose(virConnectPtr conn) virQEMUDriver *driver = conn->privateData; /* Get rid of callbacks registered for this conn */ - virCloseCallbacksRun(driver->closeCallbacks, conn, driver->domains, driver); + virCloseCallbacksRun(driver->closeCallbacks, conn, driver->domains); conn->privateData = NULL; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7de0929a36..438f2bc999 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2163,8 +2163,7 @@ qemuMigrationDstRun(virQEMUDriver *driver, */ static void qemuMigrationSrcCleanup(virDomainObj *vm, - virConnectPtr conn, - void *opaque G_GNUC_UNUSED) + virConnectPtr conn) { qemuDomainObjPrivate *priv = vm->privateData; virQEMUDriver *driver = priv->driver; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a66ca9aa60..da7f2ccaa0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8386,8 +8386,7 @@ void qemuProcessStop(virQEMUDriver *driver, static void qemuProcessAutoDestroy(virDomainObj *dom, - virConnectPtr conn, - void *opaque G_GNUC_UNUSED) + virConnectPtr conn) { qemuDomainObjPrivate *priv = dom->privateData; virQEMUDriver *driver = priv->driver; -- 2.35.3
On a Tuesday in 2022, Peter Krempa wrote:
This series simplifies some aspects of close callbacks in preparation of further refactors.
Peter Krempa (10): virclosecallbacks.h: Reformat header to contemporary style virObjectLockGuard: Require that returned value is used qemuMigrationSrcBegin: Automatically free 'xml' variable on error bhyve: Store 'driver' in VM private data lxc: Store 'driver' in VM private data qemuMigrationSrcCleanup: Use 'driver' from VM private data qemuProcessAutoDestroy: Use 'driver' from VM private data lxcProcessAutoDestroy: Use 'driver' from VM private data bhyveProcessAutoDestroy: Use 'driver' from VM private data virclosecallbacks: Don't pass opqaue pointer to callback invocation
src/bhyve/bhyve_domain.c | 8 ++++++-- src/bhyve/bhyve_domain.h | 2 ++ src/bhyve/bhyve_driver.c | 2 +- src/bhyve/bhyve_process.c | 6 +++--- src/hypervisor/virclosecallbacks.c | 5 ++--- src/hypervisor/virclosecallbacks.h | 29 +++++++++++++++++------------ src/lxc/lxc_domain.c | 4 +++- src/lxc/lxc_domain.h | 1 + src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 8 +++----- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 13 +++++-------- src/qemu/qemu_process.c | 5 ++--- src/util/virobject.h | 2 +- 14 files changed, 48 insertions(+), 41 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa