External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-07-31 05:57:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Rewrite bitmap handling for block commit

Browse Source 
Reuse qemuBlockGetBitmapMergeActions which allows removing the ad-hoc
implementation of bitmap merging for block commit. The new approach is
way simpler and more robust and also allows us to get rid of the
disabling of bitmaps done prior to the start as we actually do want to
update the bitmaps in the base.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Peter Krempa 2020-04-17 09:51:40 +02:00
parent b630cf4c0d
commit 20a7abc2d2
6 changed files with 42 additions and 272 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

203
src/qemu/qemu_block.c
View File

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

11
src/qemu/qemu_block.h
View File

@ -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,

25
src/qemu/qemu_blockjob.c
View File

@ -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. */

56
src/qemu/qemu_driver.c
View File

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

18
tests/qemublocktest.c
View File

@ -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; \

1
tests/qemublocktestdata/bitmapblockcommit/empty
View File

@ -1,2 +1 @@
pre job bitmap disable:
merge bitmpas: