[PATCH 22/32] qemu: block: Add universal helper for merging dirty bitmaps for all scenarios

Eric Blake eblake at redhat.com
Thu Jun 18 21:19:06 UTC 2020


On 6/15/20 12:10 PM, Peter Krempa wrote:
> Add a function which allows to merge bitmaps according to the new

Another reminder about "allows to $VERB" being non-idiomatic :)

s/allows to merge/allows merging/

> semantics and will allow to replace all the specific ad-hoc functions

s/allow to replace/allow replacing/

> currently in use for 'backup', 'block commit', 'block copy' and will
> also be usable in the future for 'block pull' and non-shared storage
> migration.
> 
> The semantics are a bit quirky for the 'backup' case but these quirks
> are documented and will prevent us from having two slightly different
> algorithms.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>   src/qemu/qemu_block.c | 172 ++++++++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_block.h |  10 +++
>   2 files changed, 182 insertions(+)
> 

I'll have to read the later patches that start using this in place of 
their current ad-hoc usage, but in isolation, the new function looks 
reasonable.

> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 83e3df9601..86f8410ce7 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -2847,6 +2847,178 @@ qemuBlockGetNamedNodeData(virDomainObjPtr vm,
>   }
> 
> 
> +/**
> + * qemuBlockGetBitmapMergeActionsGetBitmaps:
> + *
> + * Collect a list of bitmaps which need to be handled in
> + * qemuBlockGetBitmapMergeActions. The list contains only valid bitmaps in the
> + * sub-chain which is being processed.
> + *
> + * Note that the returned GSList contains bitmap names string pointers borrowed
> + * from @blockNamedNodeData so they must not be freed.
> + */
> +static GSList *
> +qemuBlockGetBitmapMergeActionsGetBitmaps(virStorageSourcePtr topsrc,
> +                                         const char *bitmapname,
> +                                         virHashTablePtr blockNamedNodeData)
> +{
> +    g_autoptr(GSList) ret = NULL;
> +    qemuBlockNamedNodeDataPtr entry;
> +    size_t i;
> +
> +    /* for now it doesn't make sense to consider bitmaps which are not present
> +     * in @topsrc as we can't recreate a bitmap for a layer if it's missing */

I'm guessing qemu adding block-dirty-bitmap-populate will somewhat help 
here (if it's missing in top, but present in a backing layer, the fix is 
to populate the bitmap in the top layer according to the allocation map).

> +
> +    if (!(entry = virHashLookup(blockNamedNodeData, topsrc->nodeformat)))
> +        return NULL;
> +
> +    for (i = 0; i < entry->nbitmaps; i++) {
> +        qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];
> +
> +        if (bitmapname &&
> +            STRNEQ(bitmapname, bitmap->name))
> +            continue;
> +
> +        if (!qemuBlockBitmapChainIsValid(topsrc, bitmap->name, blockNamedNodeData))
> +            continue;
> +
> +        ret = g_slist_prepend(ret, bitmap->name);
> +    }
> +
> +    return g_steal_pointer(&ret);
> +}
> +
> +
> +/**
> + * qemuBlockGetBitmapMergeActions:
> + * @topsrc: top of the chain to merge bitmaps in
> + * @basesrc: bottom of the chain to merge bitmaps in (NULL for full chain)
> + * @target: destination storage source of the merge (may be part of original chain)
> + * @bitmapname: name of bitmap to perform the merge (NULL for all bitmaps)
> + * @dstbitmapname: name of destination bitmap of the merge (see below for caveats)
> + * @writebitmapsrc: storage source corresponding to the node containing the write temporary bitmap
> + * @actions: returns actions for a 'transaction' QMP command for executing the merge
> + * @blockNamedNodeData: hash table filled with qemuBlockNamedNodeData
> + *
> + * Calculate handling of dirty block bitmaps between @topsrc and @basesrc. If
> + * @basesrc is NULL the end of the chain is considered. @target is the destination
> + * storage source definition of the merge and may or may not be part of the
> + * merged chain.
> + *
> + * Specifically the merging algorithm ensures that each considered bitmap is
> + * merged with the appropriate bitmaps so that it properly describes
> + * the state of dirty blocks when looked at from @topsrc based on the depth
> + * of the backing chain where the bitmap is placed.
> + *
> + * If @bitmapname is non-NULL only bitmaps with that name are handled, otherwise
> + * all bitmaps are considered.
> + *
> + * If @dstbitmap is non-NULL everything is merged into a bitmap with that name,
> + * otherwise each bitmap is merged into a bitmap with the same name into @target.
> + * Additionally if @dstbitmap is non-NULL the target bitmap is created as 'inactive'
> + * and 'transient' as a special case for the backup operation.
> + *
> + * If @writebitmapsrc is non-NULL, the 'libvirt-tmp-activewrite' bitmap from
> + * given node is merged along with others. This bitmap corresponds to the writes
> + * which occured between an active layer job finished and the rest of the bitmap

occurred

> + * merging.
> + *
> + * If the bitmap is not valid somehow (see qemuBlockBitmapChainIsValid) given
> + * bitmap is silently skipped, so callers must ensure that given bitmap is valid
> + * if they care about it.
> + *
> + * The resulting 'transacion' QMP command actions are filled in and returned via

transaction

> + * @actions.
> + *
> + * Note that @actions may be NULL if no merging is required.
> + */
> +int
> +qemuBlockGetBitmapMergeActions(virStorageSourcePtr topsrc,
> +                               virStorageSourcePtr basesrc,
> +                               virStorageSourcePtr target,
> +                               const char *bitmapname,
> +                               const char *dstbitmapname,
> +                               virStorageSourcePtr writebitmapsrc,
> +                               virJSONValuePtr *actions,
> +                               virHashTablePtr blockNamedNodeData)

Looks a bit odd to have an output parameter (actions) prior to an input 
parameter (blockNamedNodeData), but not fatal.

> +{
> +    g_autoptr(virJSONValue) act = virJSONValueNewArray();
> +    virStorageSourcePtr n;
> +
> +    g_autoptr(GSList) bitmaps = NULL;
> +    GSList *next;
> +
> +    if (!(bitmaps = qemuBlockGetBitmapMergeActionsGetBitmaps(topsrc, bitmapname,
> +                                                             blockNamedNodeData)))
> +        return 0;

Look up one or all bitmaps in the top layer (based on whether bitmapname 
is set),

> +
> +    for (next = bitmaps; next; next = next->next) {
> +        const char *curbitmap = next->data;
> +        const char *mergebitmapname = dstbitmapname;
> +        bool mergebitmappersistent = false;
> +        bool mergebitmapdisabled = true;
> +        g_autoptr(virJSONValue) merge = virJSONValueNewArray();
> +        unsigned long long granularity = 0;
> +        qemuBlockNamedNodeDataBitmapPtr bitmap;
> +
> +        /* explicitly named destinations mean that we want a temporary
> +         * disabled bitmap only, so undo the default for non-explicit cases  */
> +        if (!mergebitmapname) {
> +            mergebitmapname = curbitmap;
> +            mergebitmappersistent = true;
> +            mergebitmapdisabled = false;
> +        }
> +
> +        for (n = topsrc; virStorageSourceIsBacking(n) && n != basesrc; n = n->backingStore) {
> +            if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
> +                                                                 n, curbitmap)))
> +                continue;
> +
> +            if (granularity == 0)
> +                granularity = bitmap->granularity;
> +
> +            if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(merge,
> +                                                                 n->nodeformat,
> +                                                                 bitmap->name) < 0)
> +                return -1;
> +        }

So this loop merges the same bitmap name along all disks in the backing 
chain where it is present,


> +
> +        if (dstbitmapname ||
> +            !(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
> +                                                             target, curbitmap))) {
> +
> +            if (qemuMonitorTransactionBitmapAdd(act,
> +                                                target->nodeformat,
> +                                                mergebitmapname,
> +                                                mergebitmappersistent,
> +                                                mergebitmapdisabled,
> +                                                granularity) < 0)
> +                return -1;
> +        }

if a destination was set or if the bitmap does not exist in the target 
layer, then we add in a bitmap for the destination,

> +
> +        if (writebitmapsrc &&
> +            qemuMonitorTransactionBitmapMergeSourceAddBitmap(merge,
> +                                                             writebitmapsrc->nodeformat,
> +                                                             "libvirt-tmp-activewrite") < 0)
> +            return -1;

if a temporary bitmap was needed, we add that in too,

> +
> +        if (qemuMonitorTransactionBitmapMerge(act, target->nodeformat,
> +                                              mergebitmapname, &merge) < 0)
> +            return -1;

and repeat a merge inside the single transaction for each bitmap involved.

> +    }
> +
> +    if (writebitmapsrc &&
> +        qemuMonitorTransactionBitmapRemove(act, writebitmapsrc->nodeformat,
> +                                           "libvirt-tmp-activewrite") < 0)
> +        return -1;

The transaction also removes any temporary bitmap.  You only do this 
once instead of per-bitmap; I'm guessing that means that writebitmapsrc 
can only be used when bitmapname is non-NULL.  Should there be a check 
for that?

> +
> +    if (virJSONValueArraySize(act) > 0)
> +        *actions = g_steal_pointer(&act);
> +
> +    return 0;
> +}
> +
> +

Otherwise, it seems to make sense as a generalized tool.  We'll see how 
later patches use it.

Reviewed-by: Eric Blake <eblake at redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




More information about the libvir-list mailing list