[PATCH 05/13] qemu: block: Implement helpers for dealing with bitmaps during block commit

Pavel Mores pmores at redhat.com
Thu Mar 5 10:20:50 UTC 2020


On Wed, Mar 04, 2020 at 06:26:33PM +0100, Peter Krempa wrote:
> qemuBlockBitmapsHandleCommitStart prepares for disabling the bitmaps in
> the 'base' of the commit job so that the bitmaps are not dirtied by the
> commit job. This needs to be done prior to start of the commit job.
> 
> qemuBlockBitmapsHandleCommitFinish then calculates the necessary merges
> that agregate all the bitmaps between the commited images and write them
> into the base bitmap.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>  src/qemu/qemu_block.c | 217 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_block.h |  14 +++
>  2 files changed, 231 insertions(+)
> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 11df8eedd0..2467315563 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -2962,6 +2962,223 @@ 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 hanlding 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 top,

According to the documentation above and by symmetry with other changes in this
commit, shouldn't this parameter be actually called '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 = true;
> +    size_t i;
> +
> +    if (!(entry = virHashLookup(blockNamedNodeData, basesrc->nodeformat)))
> +        return 0;
> +
> +    bitmaplist = g_new0(char *, entry->nbitmaps);
> +
> +    for (i = 0; i < entry->nbitmaps; i++) {
> +        qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];
> +
> +        if (!bitmap->recording || bitmap->inconsistent ||
> +            !qemuBlockBitmapChainIsValid(top, 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) {

Can 'disable_bitmaps' actually be 'false' at this point?  Perhaps it should be
initialised to 'false' at the top of this function?

> +        *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
> + * @blockNamedNodeData: hash table containing data about bitmaps
> + * @actions: filled with arguments for a 'transaction' command
> + * @disabledBitmapsBase: bitmap names which were disabled
> + *
> + * Calculates the necessary bitmap merges/additions/enablements to properly
> + * handle commit of images from 'top' into 'base'. The necessary operations
> + * in form of argumets of the 'transaction' command are filled into 'actions'
> + * if there is anything to do. Otherwise NULL is returned.
> + */
> +int
> +qemuBlockBitmapsHandleCommitFinish(virStorageSourcePtr topsrc,
> +                                   virStorageSourcePtr basesrc,
> +                                   virHashTablePtr blockNamedNodeData,
> +                                   virJSONValuePtr *actions,
> +                                   char **disabledBitmapsBase)
> +{
> +    g_autoptr(virJSONValue) act = virJSONValueNewArray();
> +    virStorageSourcePtr n;
> +    qemuBlockNamedNodeDataPtr entry;
> +    g_autoptr(virHashTable) commitdata = NULL;
> +    struct qemuBlockBitmapsHandleCommitData *bitmapdata;
> +    size_t i;
> +
> +    commitdata = virHashNew(qemuBlockBitmapsHandleCommitDataFree);
> +
> +    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 {
> +                char **disabledbitmaps = disabledBitmapsBase;
> +
> +                for (disabledbitmaps = disabledBitmapsBase; *disabledbitmaps; disabledbitmaps++) {
> +                    if (STREQ(*disabledBitmapsBase, 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)
> +        return -1;
> +
> +    if (virJSONValueArraySize(act) > 0)
> +        *actions = g_steal_pointer(&act);
> +
> +    return 0;
> +}
> +
> +
>  /**
>   * qemuBlockReopenFormat:
>   * @vm: domain object
> diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
> index b3d7d0f876..cb408d2eee 100644
> --- a/src/qemu/qemu_block.h
> +++ b/src/qemu/qemu_block.h
> @@ -229,6 +229,20 @@ 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,
> +                                   virHashTablePtr blockNamedNodeData,
> +                                   virJSONValuePtr *actions,
> +                                   char **disabledBitmapsBase);
> +
>  int
>  qemuBlockReopenFormat(virDomainObjPtr vm,
>                        virStorageSourcePtr src,
> -- 
> 2.24.1
> 




More information about the libvir-list mailing list