From 64366c005693a2fc389e3f61dbc68259ead2c260 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Mon, 9 Jan 2023 14:01:59 +0100 Subject: [PATCH] qemu: snapshot: Restructure control flow to detect errors sooner and work around compiler Some compilers aren't happy when an automatically freed variable is used just to free something (thus it's only assigned in the code): When compiling qemuSnapshotDelete after recent commits they complain: ../src/qemu/qemu_snapshot.c:3153:61: error: variable 'delData' set but not used [-Werror,-Wunused-but-set-variable] g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL; ^ To work around the issue we can restructure the code which also has the following semantic implications: - since qemuSnapshotDeleteExternalPrepare does validation we error out sooner than attempting to start the VM - we read the temporary variable at least in one code path Fixes: 4a4d89a9252 Signed-off-by: Peter Krempa Reviewed-by: Pavel Hrdina --- src/qemu/qemu_snapshot.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 32f7011cbe..b8416808b3 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -3146,12 +3146,13 @@ qemuSnapshotDelete(virDomainObj *vm, if (virDomainSnapshotIsExternal(snap) && !(flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY))) { + g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL; - externalData = qemuSnapshotDeleteExternalPrepare(vm, snap); + /* this also serves as validation whether the snapshot can be deleted */ + if (!(tmpData = qemuSnapshotDeleteExternalPrepare(vm, snap))) + goto endjob; if (!virDomainObjIsActive(vm)) { - g_autoslist(qemuSnapshotDeleteExternalData) delData = NULL; - if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, @@ -3163,20 +3164,19 @@ qemuSnapshotDelete(virDomainObj *vm, /* Call the prepare again as some data require that the VM is * running to get everything we need. */ - delData = g_steal_pointer(&externalData); - externalData = qemuSnapshotDeleteExternalPrepare(vm, snap); + if (!(externalData = qemuSnapshotDeleteExternalPrepare(vm, snap))) + goto endjob; } else { qemuDomainJobPrivate *jobPriv = vm->job->privateData; + externalData = g_steal_pointer(&tmpData); + /* If the VM is running we need to indicate that the async snapshot * job is snapshot delete job. */ jobPriv->snapshotDelete = true; qemuDomainSaveStatus(vm); } - - if (!externalData) - goto endjob; } } -- 2.39.5