From b83e87ca78afc44352b5c7a6850acdc53513728a Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 26 Nov 2024 13:20:56 +0100
Subject: [PATCH] qemuBlockReopenAccess: Fix update of 'readonly' state

Refactors done in 24b667eeed78d2df (and also 9ec0e28e876b17df9)
broke the expected handling of the update of 'readonly' flag of a
virStorage. The source is actually set to the proper state but rolled
back to the previous state as the 'cleanup' label should have been
'error' and thus not reached on success.

Additionally some of the code paths violate the statement in the comment
after updating 'readonly' that only 'goto error' must be used.

Fixes: 24b667eeed78d2df0376a38a592ed9d8c2744bdc
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_block.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 088c128424..a7c8be8d8b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -3172,7 +3172,6 @@ qemuBlockReopenAccess(virDomainObj *vm,
     g_autoptr(virJSONValue) reopenoptions = virJSONValueNewArray();
     g_autoptr(virJSONValue) srcprops = NULL;
     int rc;
-    int ret = -1;
 
     VIR_DEBUG("nodename:'%s' current-ro:'%d requested-ro='%d'",
               qemuBlockStorageSourceGetEffectiveNodename(src),
@@ -3190,39 +3189,39 @@ qemuBlockReopenAccess(virDomainObj *vm,
     }
 
     src->readonly = readonly;
-    /* from now on all error paths must use 'goto cleanup' */
+    /* from now on all error paths must use 'goto error' which restores the original state */
 
     /* based on which is the current 'effecitve' layer we must reopen the
      * appropriate blockdev */
     if (qemuBlockStorageSourceGetFormatNodename(src)) {
         if (!(srcprops = qemuBlockStorageSourceGetFormatProps(src, src->backingStore)))
-            return -1;
+            goto error;
     } else if (qemuBlockStorageSourceGetSliceNodename(src)) {
         if (!(srcprops = qemuBlockStorageSourceGetBlockdevStorageSliceProps(src, true, false)))
-            return -1;
+            goto error;
     } else {
         if (!(srcprops = qemuBlockStorageSourceGetBackendProps(src,
                                                                QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_EFFECTIVE_NODE)))
-            return -1;
+            goto error;
     }
 
     if (virJSONValueArrayAppend(reopenoptions, &srcprops) < 0)
-        return -1;
+        goto error;
 
     if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
-        goto cleanup;
+        goto error;
 
     rc = qemuMonitorBlockdevReopen(qemuDomainGetMonitor(vm), &reopenoptions);
 
     qemuDomainObjExitMonitor(vm);
     if (rc < 0)
-        goto cleanup;
+        goto error;
 
-    ret = 0;
+    return 0;
 
- cleanup:
+ error:
     src->readonly = !readonly;
-    return ret;
+    return -1;
 }