diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index c3ed87c284..6a06436abf 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3209,117 +3209,7 @@ qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src, /** * @topsrc: virStorageSource representing 'top' of the job * @basesrc: virStorageSource representing 'base' of the job - * @blockNamedNodeData: hash table containing data about bitmaps - * @actions: filled with arguments for a 'transaction' command - * @disabledBitmapsBase: filled with a list of bitmap names which must be disabled - * - * Prepares data for correctly handling bitmaps during the start of a commit - * job. The bitmaps in the 'base' image must be disabled, so that the writes - * done by the blockjob don't dirty the enabled bitmaps. - * - * @actions and @disabledBitmapsBase are untouched if no bitmaps need - * to be disabled. - */ -int -qemuBlockBitmapsHandleCommitStart(virStorageSourcePtr topsrc, - virStorageSourcePtr basesrc, - virHashTablePtr blockNamedNodeData, - virJSONValuePtr *actions, - char ***disabledBitmapsBase) -{ - g_autoptr(virJSONValue) act = virJSONValueNewArray(); - VIR_AUTOSTRINGLIST bitmaplist = NULL; - size_t curbitmapstr = 0; - qemuBlockNamedNodeDataPtr entry; - bool disable_bitmaps = false; - size_t i; - - if (!(entry = virHashLookup(blockNamedNodeData, basesrc->nodeformat))) - return 0; - - bitmaplist = g_new0(char *, entry->nbitmaps + 1); - - for (i = 0; i < entry->nbitmaps; i++) { - qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i]; - - if (!bitmap->recording || bitmap->inconsistent || - !qemuBlockBitmapChainIsValid(topsrc, bitmap->name, blockNamedNodeData)) - continue; - - disable_bitmaps = true; - - if (qemuMonitorTransactionBitmapDisable(act, basesrc->nodeformat, - bitmap->name) < 0) - return -1; - - bitmaplist[curbitmapstr++] = g_strdup(bitmap->name); - } - - if (disable_bitmaps) { - *actions = g_steal_pointer(&act); - *disabledBitmapsBase = g_steal_pointer(&bitmaplist); - } - - return 0; -} - - -struct qemuBlockBitmapsHandleCommitData { - bool skip; - bool create; - bool enable; - const char *basenode; - virJSONValuePtr merge; - unsigned long long granularity; - bool persistent; -}; - - -static void -qemuBlockBitmapsHandleCommitDataFree(void *opaque) -{ - struct qemuBlockBitmapsHandleCommitData *data = opaque; - - virJSONValueFree(data->merge); - g_free(data); -} - - -static int -qemuBlockBitmapsHandleCommitFinishIterate(void *payload, - const void *entryname, - void *opaque) -{ - struct qemuBlockBitmapsHandleCommitData *data = payload; - const char *bitmapname = entryname; - virJSONValuePtr actions = opaque; - - if (data->skip) - return 0; - - if (data->create) { - if (qemuMonitorTransactionBitmapAdd(actions, data->basenode, bitmapname, - data->persistent, !data->enable, - data->granularity) < 0) - return -1; - } else { - if (data->enable && - qemuMonitorTransactionBitmapEnable(actions, data->basenode, bitmapname) < 0) - return -1; - } - - if (data->merge && - qemuMonitorTransactionBitmapMerge(actions, data->basenode, bitmapname, - &data->merge) < 0) - return -1; - - return 0; -} - - -/** - * @topsrc: virStorageSource representing 'top' of the job - * @basesrc: virStorageSource representing 'base' of the job + * @active: commit job is an active layer block-commit * @blockNamedNodeData: hash table containing data about bitmaps * @actions: filled with arguments for a 'transaction' command * @disabledBitmapsBase: bitmap names which were disabled @@ -3332,95 +3222,20 @@ qemuBlockBitmapsHandleCommitFinishIterate(void *payload, int qemuBlockBitmapsHandleCommitFinish(virStorageSourcePtr topsrc, virStorageSourcePtr basesrc, + bool active, virHashTablePtr blockNamedNodeData, - virJSONValuePtr *actions, - char **disabledBitmapsBase) + virJSONValuePtr *actions) { - g_autoptr(virJSONValue) act = virJSONValueNewArray(); - virStorageSourcePtr n; - qemuBlockNamedNodeDataPtr entry; - g_autoptr(virHashTable) commitdata = NULL; - struct qemuBlockBitmapsHandleCommitData *bitmapdata; - size_t i; + virStorageSourcePtr writebitmapsrc = NULL; - commitdata = virHashNew(qemuBlockBitmapsHandleCommitDataFree); + if (active) + writebitmapsrc = basesrc; - for (n = topsrc; n != basesrc; n = n->backingStore) { - if (!(entry = virHashLookup(blockNamedNodeData, n->nodeformat))) - continue; - - for (i = 0; i < entry->nbitmaps; i++) { - qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i]; - - if (!(bitmapdata = virHashLookup(commitdata, bitmap->name))) { - bitmapdata = g_new0(struct qemuBlockBitmapsHandleCommitData, 1); - - /* we must mirror the state of the topmost bitmap and merge - * everything else */ - bitmapdata->create = true; - bitmapdata->enable = bitmap->recording; - bitmapdata->basenode = basesrc->nodeformat; - bitmapdata->merge = virJSONValueNewArray(); - bitmapdata->granularity = bitmap->granularity; - bitmapdata->persistent = bitmap->persistent; - - if (virHashAddEntry(commitdata, bitmap->name, bitmapdata) < 0) { - qemuBlockBitmapsHandleCommitDataFree(bitmapdata); - return -1; - } - } - - if (bitmap->inconsistent || - !qemuBlockBitmapChainIsValid(topsrc, bitmap->name, blockNamedNodeData)) - bitmapdata->skip = true; - - if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(bitmapdata->merge, - n->nodeformat, - bitmap->name) < 0) - return -1; - } - } - - if ((entry = virHashLookup(blockNamedNodeData, basesrc->nodeformat))) { - /* note that all bitmaps in 'base' were disabled when commit was started */ - for (i = 0; i < entry->nbitmaps; i++) { - qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i]; - - if ((bitmapdata = virHashLookup(commitdata, bitmap->name))) { - bitmapdata->create = false; - } else { - if (disabledBitmapsBase) { - char **disabledbitmaps; - - for (disabledbitmaps = disabledBitmapsBase; *disabledbitmaps; disabledbitmaps++) { - if (STREQ(*disabledbitmaps, bitmap->name)) { - bitmapdata = g_new0(struct qemuBlockBitmapsHandleCommitData, 1); - - bitmapdata->create = false; - bitmapdata->enable = true; - bitmapdata->basenode = basesrc->nodeformat; - bitmapdata->granularity = bitmap->granularity; - bitmapdata->persistent = bitmap->persistent; - - if (virHashAddEntry(commitdata, bitmap->name, bitmapdata) < 0) { - qemuBlockBitmapsHandleCommitDataFree(bitmapdata); - return -1; - } - - break; - } - } - } - } - } - } - - if (virHashForEach(commitdata, qemuBlockBitmapsHandleCommitFinishIterate, act) < 0) + if (qemuBlockGetBitmapMergeActions(topsrc, basesrc, basesrc, NULL, NULL, + writebitmapsrc, actions, + blockNamedNodeData) < 0) return -1; - if (virJSONValueArraySize(act) > 0) - *actions = g_steal_pointer(&act); - return 0; } diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 0f5b4cfaa9..24b87e79db 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -243,19 +243,12 @@ qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src, bool shallow, virJSONValuePtr *actions); -int -qemuBlockBitmapsHandleCommitStart(virStorageSourcePtr topsrc, - virStorageSourcePtr basesrc, - virHashTablePtr blockNamedNodeData, - virJSONValuePtr *actions, - char ***disabledBitmapsBase); - int qemuBlockBitmapsHandleCommitFinish(virStorageSourcePtr topsrc, virStorageSourcePtr basesrc, + bool active, virHashTablePtr blockNamedNodeData, - virJSONValuePtr *actions, - char **disabledBitmapsBase); + virJSONValuePtr *actions); int qemuBlockReopenReadWrite(virDomainObjPtr vm, diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 17dc08476b..8e81b142f4 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1069,6 +1069,7 @@ qemuBlockJobProcessEventCompletedCommitBitmaps(virDomainObjPtr vm, qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(virHashTable) blockNamedNodeData = NULL; g_autoptr(virJSONValue) actions = NULL; + bool active = job->type == QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN)) return 0; @@ -1078,16 +1079,18 @@ qemuBlockJobProcessEventCompletedCommitBitmaps(virDomainObjPtr vm, if (qemuBlockBitmapsHandleCommitFinish(job->data.commit.top, job->data.commit.base, + active, blockNamedNodeData, - &actions, - job->data.commit.disabledBitmapsBase) < 0) + &actions) < 0) return 0; if (!actions) return 0; - if (qemuBlockReopenReadWrite(vm, job->data.commit.base, asyncJob) < 0) - return -1; + if (!active) { + if (qemuBlockReopenReadWrite(vm, job->data.commit.base, asyncJob) < 0) + return -1; + } if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) return -1; @@ -1097,8 +1100,10 @@ qemuBlockJobProcessEventCompletedCommitBitmaps(virDomainObjPtr vm, if (qemuDomainObjExitMonitor(priv->driver, vm) < 0) return -1; - if (qemuBlockReopenReadOnly(vm, job->data.commit.base, asyncJob) < 0) - return -1; + if (!active) { + if (qemuBlockReopenReadOnly(vm, job->data.commit.base, asyncJob) < 0) + return -1; + } return 0; } @@ -1268,6 +1273,9 @@ qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver, job->disk->src = job->data.commit.base; job->disk->src->readonly = job->data.commit.top->readonly; + if (qemuBlockJobProcessEventCompletedCommitBitmaps(vm, job, asyncJob) < 0) + return; + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, job->data.commit.top); if (job->data.commit.deleteCommittedImages) @@ -1339,6 +1347,7 @@ qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuBlockJobDataPtr job) { + g_autoptr(virJSONValue) actions = virJSONValueNewArray(); virDomainDiskDefPtr disk = job->disk; VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name, vm->def->name); @@ -1346,6 +1355,10 @@ qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver, if (!disk) return; + ignore_value(qemuMonitorTransactionBitmapRemove(actions, disk->mirror->nodeformat, + "libvirt-tmp-activewrite")); + + /* Ideally, we would make the backing chain read only again (yes, SELinux * can do that using different labels). But that is not implemented yet and * not leaking security driver metadata is more important. */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7724f79c3b..a18d7f1267 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17379,9 +17379,9 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, case QEMU_BLOCKJOB_TYPE_COPY: if (blockdev && !job->jobflagsmissing) { - g_autoptr(virHashTable) blockNamedNodeData = NULL; bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW; bool reuse = job->jobflags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT; + g_autoptr(virHashTable) blockNamedNodeData = NULL; if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) return -1; @@ -17419,16 +17419,15 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, * the bitmaps if it wasn't present thus must skip this */ if (blockdev && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN)) { - g_autoptr(virHashTable) blockNamedNodeData = NULL; - if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) - return -1; + actions = virJSONValueNewArray(); - if (qemuBlockBitmapsHandleCommitFinish(job->data.commit.top, - job->data.commit.base, - blockNamedNodeData, - &actions, - job->data.commit.disabledBitmapsBase) < 0) + if (qemuMonitorTransactionBitmapAdd(actions, + job->data.commit.base->nodeformat, + "libvirt-tmp-activewrite", + false, + false, + 0) < 0) return -1; } @@ -18549,7 +18548,6 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *nodebase = NULL; bool persistjob = false; bool blockdev = false; - g_autoptr(virJSONValue) bitmapDisableActions = NULL; VIR_AUTOSTRINGLIST bitmapDisableList = NULL; virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | @@ -18714,27 +18712,6 @@ qemuDomainBlockCommit(virDomainPtr dom, goto endjob; } - if (blockdev && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN)) { - g_autoptr(virHashTable) blockNamedNodeData = NULL; - if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) - goto endjob; - - if (qemuBlockBitmapsHandleCommitStart(topSource, baseSource, - blockNamedNodeData, - &bitmapDisableActions, - &bitmapDisableList) < 0) - goto endjob; - - /* if we don't have terminator on 'base' we can't reopen it */ - if (bitmapDisableActions && !baseSource->backingStore) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("can't handle bitmaps on unterminated backing image '%s'"), - base); - goto endjob; - } - } - if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource, baseSource, &bitmapDisableList, flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE, @@ -18759,23 +18736,6 @@ qemuDomainBlockCommit(virDomainPtr dom, !(backingPath = qemuBlockGetBackingStoreString(baseSource, false))) goto endjob; - if (bitmapDisableActions) { - int rc; - - if (qemuBlockReopenReadWrite(vm, baseSource, QEMU_ASYNC_JOB_NONE) < 0) - goto endjob; - - qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorTransaction(priv->mon, &bitmapDisableActions); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto endjob; - - if (qemuBlockReopenReadOnly(vm, baseSource, QEMU_ASYNC_JOB_NONE) < 0) - goto endjob; - - if (rc < 0) - goto endjob; - } } else { device = job->name; } diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index fd6dff82f9..5be98b9320 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -928,12 +928,11 @@ testQemuBlockBitmapBlockcommit(const void *opaque) g_autofree char *actual = NULL; g_autofree char *expectpath = NULL; - g_autoptr(virJSONValue) actionsDisable = NULL; g_autoptr(virJSONValue) actionsMerge = NULL; g_autoptr(virJSONValue) nodedatajson = NULL; g_autoptr(virHashTable) nodedata = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - VIR_AUTOSTRINGLIST bitmapsDisable = NULL; + bool active = data->top == data->chain; expectpath = g_strdup_printf("%s/%s%s", abs_srcdir, blockcommitPrefix, data->name); @@ -947,20 +946,10 @@ testQemuBlockBitmapBlockcommit(const void *opaque) return -1; } - if (qemuBlockBitmapsHandleCommitStart(data->top, data->base, nodedata, - &actionsDisable, &bitmapsDisable) < 0) - return -1; - - virBufferAddLit(&buf, "pre job bitmap disable:\n"); - - if (actionsDisable && - virJSONValueToBuffer(actionsDisable, &buf, true) < 0) - return -1; - virBufferAddLit(&buf, "merge bitmpas:\n"); - if (qemuBlockBitmapsHandleCommitFinish(data->top, data->base, nodedata, - &actionsMerge, bitmapsDisable) < 0) + if (qemuBlockBitmapsHandleCommitFinish(data->top, data->base, active, nodedata, + &actionsMerge) < 0) return -1; if (actionsMerge && @@ -1357,6 +1346,7 @@ mymain(void) #define TEST_BITMAP_BLOCKCOMMIT(testname, topimg, baseimg, ndf) \ do {\ blockbitmapblockcommitdata.name = testname; \ + blockbitmapblockcommitdata.chain = bitmapSourceChain; \ blockbitmapblockcommitdata.top = testQemuBitmapGetFakeChainEntry(bitmapSourceChain, topimg); \ blockbitmapblockcommitdata.base = testQemuBitmapGetFakeChainEntry(bitmapSourceChain, baseimg); \ blockbitmapblockcommitdata.nodedatafile = ndf; \ diff --git a/tests/qemublocktestdata/bitmapblockcommit/empty b/tests/qemublocktestdata/bitmapblockcommit/empty index bfc58f994e..9260011852 100644 --- a/tests/qemublocktestdata/bitmapblockcommit/empty +++ b/tests/qemublocktestdata/bitmapblockcommit/empty @@ -1,2 +1 @@ -pre job bitmap disable: merge bitmpas: