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@xxxxxxxxxx> --- 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; } -- 2.47.0