[libvirt] [PATCH 00/19] qemu: Add support for backups in combination with snapshots
IMPORTANT: This does NOT add support for merging the bitmaps during block jobs, so manual deletion of snapshots will corrupt the incremental metadata!. Allow combining snapshots and backups. During a snapshot we create a bitmap for any active persistent bitmap to continue tracking the bitmaps properly (as qemu doesn't support creating a dirty bitmap from the allocation map). This also means that it works only for an active VM since there also isn't a way to do it via qemu-img. Thus a workaround is to start a VM as paused if this is required for offline VMs. We also now can merge bitmaps accross the backing chains. You can fetch the changes at: git fetch https://gitlab.com/pipo.sk/libvirt.git backup-snapshots Peter Krempa (19): virsh: Add QMP command wrapping for 'qemu-monitor-command' virsh: Allow extracting 'return' section of QMP command in 'qemu-monitor-command' qemu: monitor: Extract data about dirty-bimaps in qemuMonitorBlockGetNamedNodeData qemu: monitor: Extract internals of qemuMonitorJSONBlockGetNamedNodeData tests: qemublock: Add test for bitmap detection tests: qemublocktest: Add a syntetic test case for bitmap detection qemu: Check for explicit failure of qemuBlockSnapshotAddBlockdev qemu: snapshot: Fold formatting of snapshot transaction into prepare func qemu: monitor: Add 'granularity' parameter for block-dirty-bitmap-add qemu: snapshot: Propagate active bitmaps through external snapshots tests: qemublock: Add test case for detecting bitmaps as we create snapshots qemu: backup: Return 'def' instead of 'obj' from qemuBackupBeginCollectIncrementalCheckpoints qemu: backup: Extract calculations of bitmaps to merge for incremental backup qemu: backup: Propagate bitmap metadata into qemuBackupDiskPrepareOneBitmapsChain qemu: backup: Export qemuBackupDiskPrepareOneBitmapsChain for tests tests: qemublock: Add testing of bitmap merging for incremental backups qemu: block: Introduce qemuBlockNamedNodeDataGetBitmapByName qemu: backup: Merge bitmaps accross the backing chain tests: qemublock: Add tests for cross-snapshot incremental backups docs/manpages/virsh.rst | 27 +- src/qemu/qemu_backup.c | 145 ++- src/qemu/qemu_backup.h | 7 + src/qemu/qemu_block.c | 32 + src/qemu/qemu_block.h | 5 + src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_driver.c | 84 +- src/qemu/qemu_monitor.c | 6 +- src/qemu/qemu_monitor.h | 18 +- src/qemu/qemu_monitor_json.c | 96 +- src/qemu/qemu_monitor_json.h | 6 +- tests/qemublocktest.c | 241 +++++ .../backupmerge/basic-deep-out.json | 22 + .../backupmerge/basic-flat-out.json | 6 + .../backupmerge/basic-intermediate-out.json | 10 + .../backupmerge/snapshot-deep-out.json | 38 + .../backupmerge/snapshot-flat-out.json | 6 + .../snapshot-intermediate-out.json | 14 + tests/qemublocktestdata/bitmap/basic.json | 117 +++ tests/qemublocktestdata/bitmap/basic.out | 6 + tests/qemublocktestdata/bitmap/snapshots.json | 836 ++++++++++++++++++ tests/qemublocktestdata/bitmap/snapshots.out | 14 + tests/qemublocktestdata/bitmap/synthetic.json | 118 +++ tests/qemublocktestdata/bitmap/synthetic.out | 6 + tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 76 +- 26 files changed, 1845 insertions(+), 95 deletions(-) create mode 100644 tests/qemublocktestdata/backupmerge/basic-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/basic-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/basic-intermediate-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json create mode 100644 tests/qemublocktestdata/bitmap/basic.json create mode 100644 tests/qemublocktestdata/bitmap/basic.out create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out create mode 100644 tests/qemublocktestdata/bitmap/synthetic.json create mode 100644 tests/qemublocktestdata/bitmap/synthetic.out -- 2.23.0
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper: { "execute": "COMMAND" } and optionally also: { "execute": "COMMAND", "arguments":...} For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 24 +++++++++++++++++------- tools/virsh-domain.c | 32 +++++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index e9d6deaee1..697f83e5b2 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -7434,16 +7434,26 @@ qemu-monitor-command .. code-block:: shell - qemu-monitor-command domain { [--hmp] | [--pretty] } command... + qemu-monitor-command domain { [--hmp] | [--qmp] [--pretty] } command... Send an arbitrary monitor command *command* to domain *domain* through the -qemu monitor. The results of the command will be printed on stdout. If -*--hmp* is passed, the command is considered to be a human monitor command +qemu monitor. The results of the command will be printed on stdout. + +*command* is directly passed to qemu. If more than one argument is provided for +*command*, they are concatenated with a space in between before passing the +single command to the monitor. + +If *--qmp* is passed the first argument passed as *command* is used as a QMP +command name and appropriately wrapped into a JSON block. Additionally a second +argument passed as *command* is appended as value of the 'arguments' parameter +of the QMP command verbatim. + +If *--pretty* is given, and the monitor uses QMP, then the output will be +pretty-printed. + +If *--hmp* is passed, the command is considered to be a human monitor command and libvirt will automatically convert it into QMP if needed. In that case -the result will also be converted back from QMP. If *--pretty* is given, -and the monitor uses QMP, then the output will be pretty-printed. If more -than one argument is provided for *command*, they are concatenated with a -space in between before passing the single command to the monitor. +the result will also be converted back from QMP. qemu-agent-command diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 56137bdd74..9447fa2904 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9522,6 +9522,10 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = { .type = VSH_OT_BOOL, .help = N_("pretty-print any qemu monitor protocol output") }, + {.name = "qmp", + .type = VSH_OT_BOOL, + .help = N_("wrap the 'cmd' argument in JSON wrapper for QMP") + }, {.name = "cmd", .type = VSH_OT_ARGV, .flags = VSH_OFLAG_REQ, @@ -9539,16 +9543,38 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; const vshCmdOpt *opt = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + bool qmp = vshCommandOptBool(cmd, "qmp"); VSH_EXCLUSIVE_OPTIONS("hmp", "pretty"); + VSH_EXCLUSIVE_OPTIONS("hmp", "qmp"); if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); + if (qmp) { + const char *command = NULL; + const char *arguments = NULL; - virBufferTrim(&buf, " ", -1); + if ((opt = vshCommandOptArgv(ctl, cmd, opt))) + command = opt->data; + if ((opt = vshCommandOptArgv(ctl, cmd, opt))) + arguments = opt->data; + + if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) { + vshError(ctl, "%s", _("-qmp option requires 1 or 2 arguments")); + return false; + } + + virBufferAsprintf(&buf, "{\"execute\":\"%s\"", command); + if (arguments) + virBufferAsprintf(&buf, ", \"arguments\":%s", arguments); + virBufferAddLit(&buf, "}"); + } else { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) + virBufferAsprintf(&buf, "%s ", opt->data); + + virBufferTrim(&buf, " ", -1); + } monitor_cmd = virBufferContentAndReset(&buf); -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message: virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}' as shorthand for virsh qemu-monitor-command domain '"execute":"COMMAND", "arguments":{ARGUMENTS...}}' But the sugar is indeed nice (one less layer of {} JSON).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 24 +++++++++++++++++------- tools/virsh-domain.c | 32 +++++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 10 deletions(-)
- virBufferTrim(&buf, " ", -1); + if ((opt = vshCommandOptArgv(ctl, cmd, opt))) + command = opt->data; + if ((opt = vshCommandOptArgv(ctl, cmd, opt))) + arguments = opt->data; + + if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) { + vshError(ctl, "%s", _("-qmp option requires 1 or 2 arguments")); + return false;
Should we allow concatenation and/or magic behavior based on whether the second argument starts with '{'? For example, virsh qemu-monitor-command --qmp COMMAND key1=1 'key2="str"' could be shorthand for virsh qemu-monitor-command '{"execute":"COMMAND", "arguments":{"key1":1, "key2":"str"}}' But further sugar can be a separate patch, so this one works as-is: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 12/12/19 7:16 PM, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND", "arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
Since we won't use HMP to talk to qemu ever (even the small set of HMP commands we have are wrapped inside QMP once being sent down the wire), can we not use --qmp flag at all? Just look if there's "execute" in the user's input and if not add it there. For instance: virsh qemu-monitor-command query-machines will expand to {"execute":"query-machines"} and: virsh qemu-monitor-command '{"execute":"query-machines"}' will do nothing to the user's input. Michal
On Thu, Dec 12, 2019 at 19:30:27 +0100, Michal Privoznik wrote:
On 12/12/19 7:16 PM, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND", "arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
Since we won't use HMP to talk to qemu ever (even the small set of HMP commands we have are wrapped inside QMP once being sent down the wire), can
Specifically you already must use --hmp if you ever want to use HMP.
we not use --qmp flag at all? Just look if there's "execute" in the user's input and if not add it there. For instance:
virsh qemu-monitor-command query-machines
will expand to
{"execute":"query-machines"}
At first I wanted to argue that I'd like to support passing raw unmodified commands to qemu, but in fact libvirt itself parses the string as JSON so that it can be re-wrapped with the monitor sequence field, so you have to pass in JSON anyways. Thus I'm okay with dropping the flag and deciding on whether the opening '{' is present on input.
On Fri, Dec 13, 2019 at 08:04:51 +0100, Peter Krempa wrote:
On Thu, Dec 12, 2019 at 19:30:27 +0100, Michal Privoznik wrote:
On 12/12/19 7:16 PM, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
[...]
Since we won't use HMP to talk to qemu ever (even the small set of HMP commands we have are wrapped inside QMP once being sent down the wire), can
Specifically you already must use --hmp if you ever want to use HMP.
we not use --qmp flag at all? Just look if there's "execute" in the user's input and if not add it there. For instance:
virsh qemu-monitor-command query-machines
will expand to
{"execute":"query-machines"}
At first I wanted to argue that I'd like to support passing raw unmodified commands to qemu, but in fact libvirt itself parses the string as JSON so that it can be re-wrapped with the monitor sequence field, so you have to pass in JSON anyways.
Thus I'm okay with dropping the flag and deciding on whether the opening '{' is present on input.
I thought about this for a bit and looked into the code. Currently we pass to qemu anything that parses as valid JSON. It must not be an object. This means that also things like 'true' are valid JSON and would be passed to qemu. On the other hand, qemu accepts only JSON objects in the first place. This means I'm now unsure if we want to do this without an explicit switch, because we'd lose some possibilities of passtrhough. (Not very useful ones though). Other option would be to try to parse the user-provided string as JSON and if that fails do the modification. This solution is too magic for my taste though and could have non-obvious results. If you think it's okay to lose passthrough of non-objects I'll do it, but otherwise the extra flag has to stay.
On Thu, Dec 12, 2019 at 12:16:11 -0600, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND", "arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 24 +++++++++++++++++------- tools/virsh-domain.c | 32 +++++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 10 deletions(-)
- virBufferTrim(&buf, " ", -1); + if ((opt = vshCommandOptArgv(ctl, cmd, opt))) + command = opt->data; + if ((opt = vshCommandOptArgv(ctl, cmd, opt))) + arguments = opt->data; + + if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) { + vshError(ctl, "%s", _("-qmp option requires 1 or 2 arguments")); + return false;
Should we allow concatenation and/or magic behavior based on whether the second argument starts with '{'? For example,
virsh qemu-monitor-command --qmp COMMAND key1=1 'key2="str"'
could be shorthand for
virsh qemu-monitor-command '{"execute":"COMMAND", "arguments":{"key1":1, "key2":"str"}}'
I thought about this originally, but you also need a way to expose the 'null' field and booleans. Also the necessary double quoting on strings isn't exactly user friendly given that it's the most common type of argument. Also all of the above can only support creating a flat object as argument so we still need a way to do nested objects or arrays. I agree though that it's the most common case so any nested or non-default one can stay JSON-only. Thus I'll be okay doing this, but I thin we should do something else for the strings since shell quoting is often confusing to users.
On Thu, Dec 12, 2019 at 12:16:11PM -0600, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND", "arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
This is still a pretty crude variant of what QEMU can do via the 'qmp-shell' command. I wonder if a better long term bet is to turn 'qmp-shell' into an official QEMU tool & have it integrate with libvirt. ie actually install qmo-shell into /usr/bin, give it a manpage and add a '-d DOMAIN' arg as a way to tell it to send commands to 'virsh qemu-monitor-command' instead of a UNIX socket. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, Dec 13, 2019 at 10:29:20 +0000, Daniel Berrange wrote:
On Thu, Dec 12, 2019 at 12:16:11PM -0600, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh which allows simple usage of QMP and additionally prepares also for passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND", "arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
This is still a pretty crude variant of what QEMU can do via the 'qmp-shell' command.
I wonder if a better long term bet is to turn 'qmp-shell' into an official QEMU tool & have it integrate with libvirt.
ie actually install qmo-shell into /usr/bin, give it a manpage and add a '-d DOMAIN' arg as a way to tell it to send commands to 'virsh qemu-monitor-command' instead of a UNIX socket.
I conisdered doing this at one point. The nice part would be that also the QOM fuse thing shares the connection code and finding the right thing in dom is extremely painful in many cases, so using midnight commander would be fun. Unfortunately at that point it was not super obvious where to add the feature and I had better things to do. At any rate, I think this goal is orthogonal for making qemu-monitor-command more developer-friendly in the meanwhile.
Simplify gathering the actual return value from a passed-through QMP command when using 'qemu-monitor-command' by adding '--return-value' switch which just extracts the 'return' section and alternatively reports an error if the section is not present. This simplifies gathering of some test data where the full reply would need to be trimmed just for the actual return value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 5 ++++- tools/virsh-domain.c | 44 ++++++++++++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 697f83e5b2..7ec620f6ee 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -7434,7 +7434,7 @@ qemu-monitor-command .. code-block:: shell - qemu-monitor-command domain { [--hmp] | [--qmp] [--pretty] } command... + qemu-monitor-command domain { [--hmp] | [--qmp] [--pretty] [--return-value] } command... Send an arbitrary monitor command *command* to domain *domain* through the qemu monitor. The results of the command will be printed on stdout. @@ -7451,6 +7451,9 @@ of the QMP command verbatim. If *--pretty* is given, and the monitor uses QMP, then the output will be pretty-printed. +If *--return-value* is given the 'return' key of the QMP response object is +extracted rather than passing through the full reply from qemu. + If *--hmp* is passed, the command is considered to be a human monitor command and libvirt will automatically convert it into QMP if needed. In that case the result will also be converted back from QMP. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9447fa2904..a592726042 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9526,6 +9526,10 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = { .type = VSH_OT_BOOL, .help = N_("wrap the 'cmd' argument in JSON wrapper for QMP") }, + {.name = "return-value", + .type = VSH_OT_BOOL, + .help = N_("extract the value of the 'return' key from the returned string") + }, {.name = "cmd", .type = VSH_OT_ARGV, .flags = VSH_OFLAG_REQ, @@ -9540,13 +9544,19 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) g_autoptr(virshDomain) dom = NULL; g_autofree char *monitor_cmd = NULL; g_autofree char *result = NULL; + g_autoptr(virJSONValue) resultjson = NULL; unsigned int flags = 0; const vshCmdOpt *opt = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; bool qmp = vshCommandOptBool(cmd, "qmp"); + bool pretty = vshCommandOptBool(cmd, "pretty"); + bool returnval = vshCommandOptBool(cmd, "return-value"); + virJSONValuePtr formatjson; + g_autofree char *jsonstr = NULL; VSH_EXCLUSIVE_OPTIONS("hmp", "pretty"); VSH_EXCLUSIVE_OPTIONS("hmp", "qmp"); + VSH_EXCLUSIVE_OPTIONS("hmp", "return-value"); if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -9584,17 +9594,33 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0) return false; - if (vshCommandOptBool(cmd, "pretty")) { - char *tmp; - if ((tmp = virJSONStringReformat(result, true))) { - VIR_FREE(result); - result = tmp; - virTrimSpaces(result, NULL); - } else { - vshResetLibvirtError(); + if (returnval || pretty) { + resultjson = virJSONValueFromString(result); + + if (returnval && !resultjson) { + vshError(ctl, "failed to parse JSON returned by qemu"); + return false; } } - vshPrint(ctl, "%s\n", result); + + /* print raw non-prettified result */ + if (!resultjson) { + vshPrint(ctl, "%s\n", result); + return true; + } + + if (returnval) { + if (!(formatjson = virJSONValueObjectGet(resultjson, "return"))) { + vshError(ctl, "'return' member missing"); + return false; + } + } else { + formatjson = resultjson; + } + + jsonstr = virJSONValueToString(formatjson, pretty); + virTrimSpaces(jsonstr, NULL); + vshPrint(ctl, "%s", jsonstr); return true; } -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Simplify gathering the actual return value from a passed-through QMP command when using 'qemu-monitor-command' by adding '--return-value' switch which just extracts the 'return' section and alternatively reports an error if the section is not present.
This simplifies gathering of some test data where the full reply would need to be trimmed just for the actual return value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 5 ++++- tools/virsh-domain.c | 44 ++++++++++++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 10 deletions(-)
Nice use-case. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
We will need to inspect the presence and attributes for dirty bitmaps. Extract them when processing reply of query-named-block-nodes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 15 ++++++++ src/qemu/qemu_monitor_json.c | 74 ++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 79e078fca4..1c990923d6 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -676,12 +676,27 @@ int qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, virHashTablePtr stats) ATTRIBUTE_NONNULL(2); +typedef struct _qemuBlockNamedNodeDataBitmap qemuBlockNamedNodeDataBitmap; +typedef qemuBlockNamedNodeDataBitmap *qemuBlockNamedNodeDataBitmapPtr; +struct _qemuBlockNamedNodeDataBitmap { + char *name; + bool recording; + bool busy; + bool persistent; + bool inconsistent; + + unsigned long long dirtybytes; + unsigned long long granularity; +}; typedef struct _qemuBlockNamedNodeData qemuBlockNamedNodeData; typedef qemuBlockNamedNodeData *qemuBlockNamedNodeDataPtr; struct _qemuBlockNamedNodeData { unsigned long long capacity; unsigned long long physical; + + qemuBlockNamedNodeDataBitmapPtr *bitmaps; + size_t nbitmaps; }; virHashTablePtr diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 00e1d3ce15..e3a6e3a6a2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2873,14 +2873,84 @@ qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, } +static void +qemuMonitorJSONBlockNamedNodeDataBitmapFree(qemuBlockNamedNodeDataBitmapPtr bitmap) +{ + if (!bitmap) + return; + + g_free(bitmap->name); + g_free(bitmap); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNamedNodeDataBitmap, + qemuMonitorJSONBlockNamedNodeDataBitmapFree); + + static void qemuMonitorJSONBlockNamedNodeDataFree(qemuBlockNamedNodeDataPtr data) { + size_t i; + + if (!data) + return; + + for (i = 0; i < data->nbitmaps; i++) + qemuMonitorJSONBlockNamedNodeDataBitmapFree(data->bitmaps[i]); + g_free(data->bitmaps); g_free(data); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNamedNodeData, qemuMonitorJSONBlockNamedNodeDataFree); +static qemuBlockNamedNodeDataBitmapPtr +qemuMonitorJSONBlockGetNamedNodeDataBitmapOne(virJSONValuePtr val) +{ + g_autoptr(qemuBlockNamedNodeDataBitmap) bitmap = NULL; + const char *name; + + bitmap = g_new0(qemuBlockNamedNodeDataBitmap, 1); + + if (!(name = virJSONValueObjectGetString(val, "name"))) + return NULL; + + bitmap->name = g_strdup(name); + + ignore_value(virJSONValueObjectGetBoolean(val, "recording", &bitmap->recording)); + ignore_value(virJSONValueObjectGetBoolean(val, "persistent", &bitmap->persistent)); + ignore_value(virJSONValueObjectGetBoolean(val, "busy", &bitmap->busy)); + ignore_value(virJSONValueObjectGetBoolean(val, "inconsistent", &bitmap->inconsistent)); + ignore_value(virJSONValueObjectGetNumberUlong(val, "granularity", &bitmap->granularity)); + ignore_value(virJSONValueObjectGetNumberUlong(val, "count", &bitmap->dirtybytes)); + + return g_steal_pointer(&bitmap); +} + + +static void +qemuMonitorJSONBlockGetNamedNodeDataBitmaps(virJSONValuePtr bitmaps, + qemuBlockNamedNodeDataPtr data) +{ + size_t nbitmaps = virJSONValueArraySize(bitmaps); + size_t i; + + data->bitmaps = g_new0(qemuBlockNamedNodeDataBitmapPtr, nbitmaps); + + for (i = 0; i < nbitmaps; i++) { + virJSONValuePtr bitmap = virJSONValueArrayGet(bitmaps, i); + qemuBlockNamedNodeDataBitmapPtr tmp; + + if (!bitmap) + continue; + + if (!(tmp = qemuMonitorJSONBlockGetNamedNodeDataBitmapOne(bitmap))) + continue; + + data->bitmaps[data->nbitmaps++] = tmp; + } +} + + static int qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, virJSONValuePtr val, @@ -2888,6 +2958,7 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, { virHashTablePtr nodes = opaque; virJSONValuePtr img; + virJSONValuePtr bitmaps; const char *nodename; g_autoptr(qemuBlockNamedNodeData) ent = NULL; @@ -2904,6 +2975,9 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, if (virJSONValueObjectGetNumberUlong(img, "actual-size", &ent->physical) < 0) ent->physical = ent->capacity; + if ((bitmaps = virJSONValueObjectGetArray(val, "dirty-bitmaps"))) + qemuMonitorJSONBlockGetNamedNodeDataBitmaps(bitmaps, ent); + if (virHashAddEntry(nodes, nodename, ent) < 0) return -1; -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
We will need to inspect the presence and attributes for dirty bitmaps. Extract them when processing reply of query-named-block-nodes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 15 ++++++++ src/qemu/qemu_monitor_json.c | 74 ++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)
+static void +qemuMonitorJSONBlockGetNamedNodeDataBitmaps(virJSONValuePtr bitmaps, + qemuBlockNamedNodeDataPtr data) +{ + size_t nbitmaps = virJSONValueArraySize(bitmaps); + size_t i; + + data->bitmaps = g_new0(qemuBlockNamedNodeDataBitmapPtr, nbitmaps); + + for (i = 0; i < nbitmaps; i++) { + virJSONValuePtr bitmap = virJSONValueArrayGet(bitmaps, i); + qemuBlockNamedNodeDataBitmapPtr tmp; + + if (!bitmap) + continue;
Can bitmap ever be NULL? (We could assert that it is not, given our correct usage of the API - except that we aren't using asserts). But doesn't hurt to leave the check in. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Dec 12, 2019 at 12:30:54 -0600, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
We will need to inspect the presence and attributes for dirty bitmaps. Extract them when processing reply of query-named-block-nodes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 15 ++++++++ src/qemu/qemu_monitor_json.c | 74 ++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)
+static void +qemuMonitorJSONBlockGetNamedNodeDataBitmaps(virJSONValuePtr bitmaps, + qemuBlockNamedNodeDataPtr data) +{ + size_t nbitmaps = virJSONValueArraySize(bitmaps); + size_t i; + + data->bitmaps = g_new0(qemuBlockNamedNodeDataBitmapPtr, nbitmaps); + + for (i = 0; i < nbitmaps; i++) { + virJSONValuePtr bitmap = virJSONValueArrayGet(bitmaps, i); + qemuBlockNamedNodeDataBitmapPtr tmp; + + if (!bitmap) + continue;
Can bitmap ever be NULL? (We could assert that it is not, given our correct usage of the API - except that we aren't using asserts). But doesn't hurt to leave the check in.
AFAIK the JSON parser guarantees that it's not NULL, but given that the return value of virJSONValueArrayGet is checked in many places I think it would make static analyzers think it might return NULL and start moaning.
Reviewed-by: Eric Blake <eblake@redhat.com>
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
For testing purposes it will be beneficial to be able to parse the data from JSON directly rather than trying to simulate the monitor. Extract the worker bits and export them. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 18 +++++++++++++----- src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e3a6e3a6a2..856c2c2778 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2993,14 +2993,10 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, virHashTablePtr -qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon) +qemuMonitorJSONBlockGetNamedNodeDataJSON(virJSONValuePtr nodes) { - g_autoptr(virJSONValue) nodes = NULL; g_autoptr(virHashTable) ret = NULL; - if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon))) - return NULL; - if (!(ret = virHashNew((virHashDataFree) qemuMonitorJSONBlockNamedNodeDataFree))) return NULL; @@ -3013,6 +3009,18 @@ qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon) } +virHashTablePtr +qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon) +{ + g_autoptr(virJSONValue) nodes = NULL; + + if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon))) + return NULL; + + return qemuMonitorJSONBlockGetNamedNodeDataJSON(nodes); +} + + int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 5d05772fa2..44926464b9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -87,6 +87,9 @@ int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, int qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, virHashTablePtr stats); +virHashTablePtr +qemuMonitorJSONBlockGetNamedNodeDataJSON(virJSONValuePtr nodes); + virHashTablePtr qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon); -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
For testing purposes it will be beneficial to be able to parse the data from JSON directly rather than trying to simulate the monitor. Extract the worker bits and export them.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 18 +++++++++++++----- src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 16 insertions(+), 5 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Test the extraction of data about changed block tracking bitmaps. The first test case adds a simple scenario of multiple bitmaps in one layer. The test data will be also later reused for testing the code that determines which bitmaps to merge for an incremental backup. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 75 ++++++++++++++ tests/qemublocktestdata/bitmap/basic.json | 117 ++++++++++++++++++++++ tests/qemublocktestdata/bitmap/basic.out | 6 ++ 3 files changed, 198 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/basic.json create mode 100644 tests/qemublocktestdata/bitmap/basic.out diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 2c170548ec..16bee47a12 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -27,6 +27,7 @@ # include "virlog.h" # include "qemu/qemu_block.h" # include "qemu/qemu_qapi.h" +# include "qemu/qemu_monitor_json.h" # include "qemu/qemu_command.h" @@ -492,6 +493,71 @@ testQemuDiskXMLToPropsValidateFileSrcOnly(const void *opaque) } +static const char *bitmapDetectPrefix = "qemublocktestdata/bitmap/"; + +static void +testQemuDetectBitmapsWorker(virHashTablePtr nodedata, + const char *nodename, + virBufferPtr buf) +{ + qemuBlockNamedNodeDataPtr data; + size_t i; + + if (!(data = virHashLookup(nodedata, nodename))) + return; + + virBufferAsprintf(buf, "%s:\n", nodename); + virBufferAdjustIndent(buf, 1); + + for (i = 0; i < data->nbitmaps; i++) { + qemuBlockNamedNodeDataBitmapPtr bitmap = data->bitmaps[i]; + + virBufferAsprintf(buf, "%8s: recod:%d busy:%d persist:%d inconist:%d gran:%llu dirty:%llu\n", + bitmap->name, bitmap->recording, bitmap->busy, + bitmap->persistent, bitmap->inconsistent, + bitmap->granularity, bitmap->dirtybytes); + } + + virBufferAdjustIndent(buf, -1); +} + + +static int +testQemuDetectBitmaps(const void *opaque) +{ + const char *name = opaque; + g_autoptr(virJSONValue) nodedatajson = NULL; + g_autoptr(virHashTable) nodedata = NULL; + g_autofree char *actual = NULL; + g_autofree char *expectpath = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + size_t i; + + expectpath = g_strdup_printf("%s/%s%s.out", abs_srcdir, + bitmapDetectPrefix, name); + + if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, name, + ".json", NULL))) + return -1; + + if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) { + VIR_TEST_VERBOSE("failed to load nodedata JSON\n"); + return -1; + } + + /* we detect for the first 30 nodenames for simplicity */ + for (i = 0; i < 30; i++) { + g_autofree char *nodename = g_strdup_printf("libvirt-%zu-format", i); + + testQemuDetectBitmapsWorker(nodedata, nodename, &buf); + } + + actual = virBufferContentAndReset(&buf); + + return virTestCompareToFile(actual, expectpath); +} + + static int mymain(void) { @@ -702,6 +768,15 @@ mymain(void) TEST_IMAGE_CREATE("network-ssh-qcow2", NULL); TEST_IMAGE_CREATE("network-sheepdog-qcow2", NULL); +# define TEST_BITMAP_DETECT(testname) \ + do { \ + if (virTestRun("bitmap detect " testname, \ + testQemuDetectBitmaps, testname) < 0) \ + ret = -1; \ + } while (0) + + TEST_BITMAP_DETECT("basic"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/bitmap/basic.json b/tests/qemublocktestdata/bitmap/basic.json new file mode 100644 index 0000000000..9d418b1a37 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/basic.json @@ -0,0 +1,117 @@ +[ + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 200704, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-format", + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "dirty-bitmaps": [ + { + "name": "current", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + }, + { + "name": "d", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "c", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "b", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "a", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.qcow2", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 197120, + "filename": "/tmp/pull4.qcow2", + "format": "file", + "actual-size": 200704, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.qcow2", + "encryption_key_missing": false + } +] diff --git a/tests/qemublocktestdata/bitmap/basic.out b/tests/qemublocktestdata/bitmap/basic.out new file mode 100644 index 0000000000..ab1660be50 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/basic.out @@ -0,0 +1,6 @@ +libvirt-1-format: + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + d: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + c: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + b: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + a: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Test the extraction of data about changed block tracking bitmaps. The first test case adds a simple scenario of multiple bitmaps in one layer.
The test data will be also later reused for testing the code that determines which bitmaps to merge for an incremental backup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 75 ++++++++++++++
+static void +testQemuDetectBitmapsWorker(virHashTablePtr nodedata, + const char *nodename, + virBufferPtr buf) +{ + qemuBlockNamedNodeDataPtr data; + size_t i; + + if (!(data = virHashLookup(nodedata, nodename))) + return; + + virBufferAsprintf(buf, "%s:\n", nodename); + virBufferAdjustIndent(buf, 1); + + for (i = 0; i < data->nbitmaps; i++) { + qemuBlockNamedNodeDataBitmapPtr bitmap = data->bitmaps[i]; + + virBufferAsprintf(buf, "%8s: recod:%d busy:%d persist:%d inconist:%d gran:%llu dirty:%llu\n",
s/recod/record/ s/inconist/inconsist/
+ + if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) { + VIR_TEST_VERBOSE("failed to load nodedata JSON\n");
Drop the \n (most VIR_TEST_VERBOSE() does not use it)
+++ b/tests/qemublocktestdata/bitmap/basic.json @@ -0,0 +1,117 @@ +[ + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2",
It might be fun to record in the commit message how you created this temp file. But not strictly necessary.
+++ b/tests/qemublocktestdata/bitmap/basic.out @@ -0,0 +1,6 @@ +libvirt-1-format: + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + d: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + c: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + b: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + a: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
Fallout here when you fix the earlier output line. With typos fixed, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
The real data gathered for the 'basic' test case don't excercise some fields. Add a copy with a few values modified. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + tests/qemublocktestdata/bitmap/synthetic.json | 118 ++++++++++++++++++ tests/qemublocktestdata/bitmap/synthetic.out | 6 + 3 files changed, 125 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/synthetic.json create mode 100644 tests/qemublocktestdata/bitmap/synthetic.out diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 16bee47a12..e3aee03c81 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -776,6 +776,7 @@ mymain(void) } while (0) TEST_BITMAP_DETECT("basic"); + TEST_BITMAP_DETECT("synthetic"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/bitmap/synthetic.json b/tests/qemublocktestdata/bitmap/synthetic.json new file mode 100644 index 0000000000..56882bd615 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/synthetic.json @@ -0,0 +1,118 @@ +[ + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 200704, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-format", + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "dirty-bitmaps": [ + { + "name": "current", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + }, + { + "name": "d", + "recording": false, + "persistent": false, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "c", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 1234, + "count": 0 + }, + { + "name": "b", + "recording": false, + "persistent": true, + "busy": true, + "status": "disabled", + "granularity": 65536, + "count": 21314 + }, + { + "name": "a", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "inconsistent": true, + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.qcow2", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 197120, + "filename": "/tmp/pull4.qcow2", + "format": "file", + "actual-size": 200704, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.qcow2", + "encryption_key_missing": false + } +] diff --git a/tests/qemublocktestdata/bitmap/synthetic.out b/tests/qemublocktestdata/bitmap/synthetic.out new file mode 100644 index 0000000000..d941716d4b --- /dev/null +++ b/tests/qemublocktestdata/bitmap/synthetic.out @@ -0,0 +1,6 @@ +libvirt-1-format: + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + d: recod:0 busy:0 persist:0 inconist:0 gran:65536 dirty:0 + c: recod:0 busy:0 persist:1 inconist:0 gran:1234 dirty:0 + b: recod:0 busy:1 persist:1 inconist:0 gran:65536 dirty:21314 + a: recod:0 busy:0 persist:1 inconist:1 gran:65536 dirty:0 -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
The real data gathered for the 'basic' test case don't excercise some
exercise
fields. Add a copy with a few values modified.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
+++ b/tests/qemublocktestdata/bitmap/synthetic.out @@ -0,0 +1,6 @@ +libvirt-1-format: + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + d: recod:0 busy:0 persist:0 inconist:0 gran:65536 dirty:0 + c: recod:0 busy:0 persist:1 inconist:0 gran:1234 dirty:0 + b: recod:0 busy:1 persist:1 inconist:0 gran:65536 dirty:21314 + a: recod:0 busy:0 persist:1 inconist:1 gran:65536 dirty:0
Fallout after typo fixes in 5/19. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Check that the value is less than 0. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4d77fd6f6a..41a124d215 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15533,7 +15533,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (blockdev) { if (qemuBlockSnapshotAddBlockdev(actions, diskdata[i].disk, - diskdata[i].src)) + diskdata[i].src) < 0) goto cleanup; } else { if (qemuBlockSnapshotAddLegacy(actions, -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Check that the value is less than 0.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4d77fd6f6a..41a124d215 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15533,7 +15533,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (blockdev) { if (qemuBlockSnapshotAddBlockdev(actions, diskdata[i].disk, - diskdata[i].src)) + diskdata[i].src) < 0)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
qemuDomainSnapshotDiskPrepareOne is already called for each disk which is member of the snapshot so we don't need to iterate through the snapshot list again to generate members of the 'transaction' command for each snapshot. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 45 +++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 41a124d215..d2769dab1a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15285,7 +15285,8 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, virHashTablePtr blockNamedNodeData, bool reuse, bool blockdev, - qemuDomainAsyncJob asyncJob) + qemuDomainAsyncJob asyncJob, + virJSONValuePtr actions) { virDomainDiskDefPtr persistdisk; bool supportsCreate; @@ -15358,10 +15359,17 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, dd->prepared = true; - if (blockdev && - qemuDomainSnapshotDiskPrepareOneBlockdev(driver, vm, dd, cfg, reuse, - blockNamedNodeData, asyncJob) < 0) - return -1; + if (blockdev) { + if (qemuDomainSnapshotDiskPrepareOneBlockdev(driver, vm, dd, cfg, reuse, + blockNamedNodeData, asyncJob) < 0) + return -1; + + if (qemuBlockSnapshotAddBlockdev(actions, dd->disk, dd->src) < 0) + return -1; + } else { + if (qemuBlockSnapshotAddLegacy(actions, dd->disk, dd->src, reuse) < 0) + return -1; + } return 0; } @@ -15383,7 +15391,8 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, virHashTablePtr blockNamedNodeData, qemuDomainAsyncJob asyncJob, qemuDomainSnapshotDiskDataPtr *rdata, - size_t *rndata) + size_t *rndata, + virJSONValuePtr actions) { size_t i; qemuDomainSnapshotDiskDataPtr data; @@ -15403,7 +15412,8 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, data + ndata++, blockNamedNodeData, reuse, blockdev, - asyncJob) < 0) + asyncJob, + actions) < 0) goto cleanup; } @@ -15516,7 +15526,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * have to roll back later */ if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, cfg, reuse, blockdev, blockNamedNodeData, asyncJob, - &diskdata, &ndiskdata) < 0) + &diskdata, &ndiskdata, actions) < 0) goto cleanup; /* check whether there's anything to do */ @@ -15525,25 +15535,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, goto cleanup; } - /* Based on earlier qemuDomainSnapshotPrepare, all disks in this list are - * now either VIR_DOMAIN_SNAPSHOT_LOCATION_NONE, or - * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and - * qcow2 format. */ - for (i = 0; i < ndiskdata; i++) { - if (blockdev) { - if (qemuBlockSnapshotAddBlockdev(actions, - diskdata[i].disk, - diskdata[i].src) < 0) - goto cleanup; - } else { - if (qemuBlockSnapshotAddLegacy(actions, - diskdata[i].disk, - diskdata[i].src, - reuse) < 0) - goto cleanup; - } - } - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
qemuDomainSnapshotDiskPrepareOne is already called for each disk which is member of the snapshot so we don't need to iterate through the snapshot list again to generate members of the 'transaction' command for each snapshot.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 45 +++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 27 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 4 ++-- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_monitor.c | 6 ++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 7 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index c9709dc29a..5c9747f09d 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -201,7 +201,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, dd->domdisk->src->nodeformat, dd->incrementalBitmap, false, - true) < 0) + true, 0) < 0) return -1; if (qemuMonitorTransactionBitmapMerge(actions, @@ -214,7 +214,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, dd->store->nodeformat, dd->incrementalBitmap, false, - true) < 0) + true, 0) < 0) return -1; if (qemuMonitorTransactionBitmapMerge(actions, diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 38638c3b1e..97bc97bb8e 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -309,7 +309,7 @@ qemuCheckpointAddActions(virDomainObjPtr vm, if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP) continue; node = qemuDomainDiskNodeFormatLookup(vm, disk->name); - if (qemuMonitorTransactionBitmapAdd(actions, node, disk->bitmap, true, false) < 0) + if (qemuMonitorTransactionBitmapAdd(actions, node, disk->bitmap, true, false, 0) < 0) return -1; /* We only want one active bitmap for a disk along the diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ea3e62dc8e..ccd20b3740 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4543,9 +4543,11 @@ qemuMonitorTransactionBitmapAdd(virJSONValuePtr actions, const char *node, const char *name, bool persistent, - bool disabled) + bool disabled, + unsigned long long granularity) { - return qemuMonitorJSONTransactionBitmapAdd(actions, node, name, persistent, disabled); + return qemuMonitorJSONTransactionBitmapAdd(actions, node, name, persistent, + disabled, granularity); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 1c990923d6..3f3b81cddd 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1374,7 +1374,8 @@ qemuMonitorTransactionBitmapAdd(virJSONValuePtr actions, const char *node, const char *name, bool persistent, - bool disabled); + bool disabled, + unsigned long long granularity); int qemuMonitorTransactionBitmapRemove(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 856c2c2778..4e1bcaa30d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9159,7 +9159,8 @@ qemuMonitorJSONTransactionBitmapAdd(virJSONValuePtr actions, const char *node, const char *name, bool persistent, - bool disabled) + bool disabled, + unsigned long long granularity) { return qemuMonitorJSONTransactionAdd(actions, "block-dirty-bitmap-add", @@ -9167,6 +9168,7 @@ qemuMonitorJSONTransactionBitmapAdd(virJSONValuePtr actions, "s:name", name, "b:persistent", persistent, "b:disabled", disabled, + "P:granularity", granularity, NULL); } diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 44926464b9..61f5b0061d 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -636,7 +636,8 @@ qemuMonitorJSONTransactionBitmapAdd(virJSONValuePtr actions, const char *node, const char *name, bool persistent, - bool disabled); + bool disabled, + unsigned long long granularity); int qemuMonitorJSONTransactionBitmapRemove(virJSONValuePtr actions, const char *node, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 4f3bfad1d7..3b0f85e7c9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2956,7 +2956,7 @@ testQemuMonitorJSONTransaction(const void *opaque) qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, "node2", "bitmap2") < 0) return -1; - if (qemuMonitorTransactionBitmapAdd(actions, "node1", "bitmap1", true, true) < 0 || + if (qemuMonitorTransactionBitmapAdd(actions, "node1", "bitmap1", true, true, 1234) < 0 || qemuMonitorTransactionBitmapRemove(actions, "node2", "bitmap2") < 0 || qemuMonitorTransactionBitmapEnable(actions, "node3", "bitmap3") < 0 || qemuMonitorTransactionBitmapDisable(actions, "node4", "bitmap4") < 0 || -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 4 ++-- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_monitor.c | 6 ++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 7 files changed, 15 insertions(+), 9 deletions(-)
Not quite sure where you will use it, but it doesn't hurt to add. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re-create any active persistent bitmap in the snapshot overlay image so that tracking for a checkpoint is persisted. While this basically duplicates data in the allocation map it's currently the only possible way as qemu can't mirror the allocation map into a dirty bitmap if we'd ever want to do a backup. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d2769dab1a..8ccd6b7c97 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15223,6 +15223,42 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, } +/** + * qemuDomainSnapshotDiskBitmapsPropagate: + * + * This function propagates any active persistent bitmap present in the original + * image into the new snapshot. We leave the original bitmap active as in cases + * when the overlay is discarded (snapshot revert with abandoning the history) + * everything works as expected. + * */ +static int +qemuDomainSnapshotDiskBitmapsPropagate(qemuDomainSnapshotDiskDataPtr dd, + virJSONValuePtr actions, + virHashTablePtr blockNamedNodeData) +{ + qemuBlockNamedNodeDataPtr entry; + size_t i; + + if (!(entry = virHashLookup(blockNamedNodeData, dd->disk->src->nodeformat))) + return 0; + + for (i = 0; i < entry->nbitmaps; i++) { + qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i]; + + /* we don't care about temporary, inconsistent, or disabled bitmaps */ + if (!bitmap->persistent || !bitmap->recording || bitmap->inconsistent) + continue; + + if (qemuMonitorTransactionBitmapAdd(actions, dd->src->nodeformat, + bitmap->name, true, false, + bitmap->granularity) < 0) + return -1; + } + + return 0; +} + + static int qemuDomainSnapshotDiskPrepareOneBlockdev(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -15364,6 +15400,9 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, blockNamedNodeData, asyncJob) < 0) return -1; + if (qemuDomainSnapshotDiskBitmapsPropagate(dd, actions, blockNamedNodeData) < 0) + return -1; + if (qemuBlockSnapshotAddBlockdev(actions, dd->disk, dd->src) < 0) return -1; } else { -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Re-create any active persistent bitmap in the snapshot overlay image so that tracking for a checkpoint is persisted. While this basically duplicates data in the allocation map it's currently the only possible way as qemu can't mirror the allocation map into a dirty bitmap if we'd ever want to do a backup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d2769dab1a..8ccd6b7c97 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15223,6 +15223,42 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, }
+/** + * qemuDomainSnapshotDiskBitmapsPropagate: + * + * This function propagates any active persistent bitmap present in the original + * image into the new snapshot. We leave the original bitmap active as in cases + * when the overlay is discarded (snapshot revert with abandoning the history) + * everything works as expected.
That, and the backing image is now read-only so the active bitmap in that layer won't be getting any more writes until you merge the active layer back in via commit.
+ * */
Funny looking comment end.
+static int +qemuDomainSnapshotDiskBitmapsPropagate(qemuDomainSnapshotDiskDataPtr dd, + virJSONValuePtr actions, + virHashTablePtr blockNamedNodeData) +{ + qemuBlockNamedNodeDataPtr entry; + size_t i; + + if (!(entry = virHashLookup(blockNamedNodeData, dd->disk->src->nodeformat))) + return 0; + + for (i = 0; i < entry->nbitmaps; i++) { + qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i]; + + /* we don't care about temporary, inconsistent, or disabled bitmaps */ + if (!bitmap->persistent || !bitmap->recording || bitmap->inconsistent) + continue; + + if (qemuMonitorTransactionBitmapAdd(actions, dd->src->nodeformat, + bitmap->name, true, false, + bitmap->granularity) < 0)
Ah, you're reproducing whatever granularity already existed (even though so far, libvirt doesn't expose a knob for setting checkpoint bitmap granularity, so it will always be the default unless the image was modified behind libvirt's back). Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Add test data gathered from a run of qemu after creating bitmaps and snapshots together in various combinations. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + tests/qemublocktestdata/bitmap/snapshots.json | 836 ++++++++++++++++++ tests/qemublocktestdata/bitmap/snapshots.out | 14 + 3 files changed, 851 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index e3aee03c81..25afef46bb 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -777,6 +777,7 @@ mymain(void) TEST_BITMAP_DETECT("basic"); TEST_BITMAP_DETECT("synthetic"); + TEST_BITMAP_DETECT("snapshots"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/bitmap/snapshots.json b/tests/qemublocktestdata/bitmap/snapshots.json new file mode 100644 index 0000000000..87e77ad408 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots.json @@ -0,0 +1,836 @@ +[ + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "backing-image": { + "backing-image": { + "backing-image": { + "backing-image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911522", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.qcow2", + "backing-filename": "/tmp/pull4.qcow2", + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911527", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 217088, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "c", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "b", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.1575911522", + "backing-filename": "/tmp/pull4.1575911522", + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911540", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 212992, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "d", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "c", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.1575911527", + "backing-filename": "/tmp/pull4.1575911527", + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911550", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 212992, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "in-use", + "auto" + ], + "name": "current", + "granularity": 65536 + }, + { + "flags": [ + "in-use" + ], + "name": "d", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.1575911540", + "backing-filename": "/tmp/pull4.1575911540", + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-format", + "backing_file_depth": 4, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "backing_file": "/tmp/pull4.1575911540", + "dirty-bitmaps": [ + { + "name": "d", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "current", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911550", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 393728, + "filename": "/tmp/pull4.1575911550", + "format": "file", + "actual-size": 212992, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-1-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911550", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "backing-image": { + "backing-image": { + "backing-image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911522", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.qcow2", + "backing-filename": "/tmp/pull4.qcow2", + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911527", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 217088, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "c", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "b", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.1575911522", + "backing-filename": "/tmp/pull4.1575911522", + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911540", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 212992, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "d", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "c", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.1575911527", + "backing-filename": "/tmp/pull4.1575911527", + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "libvirt-2-format", + "backing_file_depth": 3, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "backing_file": "/tmp/pull4.1575911527", + "dirty-bitmaps": [ + { + "name": "c", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "d", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911540", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 393728, + "filename": "/tmp/pull4.1575911540", + "format": "file", + "actual-size": 212992, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-2-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911540", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "backing-image": { + "backing-image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911522", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.qcow2", + "backing-filename": "/tmp/pull4.qcow2", + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911527", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 217088, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "c", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "b", + "granularity": 65536 + }, + { + "flags": [ + + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.1575911522", + "backing-filename": "/tmp/pull4.1575911522", + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "libvirt-3-format", + "backing_file_depth": 2, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "backing_file": "/tmp/pull4.1575911522", + "dirty-bitmaps": [ + { + "name": "a", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "b", + "recording": false, + "persistent": true, + "busy": false, + "status": "disabled", + "granularity": 65536, + "count": 0 + }, + { + "name": "c", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911527", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 459264, + "filename": "/tmp/pull4.1575911527", + "format": "file", + "actual-size": 217088, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-3-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911527", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "backing-image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "backing-filename-format": "qcow2", + "virtual-size": 10485760, + "filename": "/tmp/pull4.1575911522", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "/tmp/pull4.qcow2", + "backing-filename": "/tmp/pull4.qcow2", + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "libvirt-4-format", + "backing_file_depth": 1, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "backing_file": "/tmp/pull4.qcow2", + "dirty-bitmaps": [ + { + "name": "a", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911522", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 328192, + "filename": "/tmp/pull4.1575911522", + "format": "file", + "actual-size": 208896, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-4-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.1575911522", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 10485760, + "filename": "/tmp/pull4.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 208896, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + { + "flags": [ + "auto" + ], + "name": "a", + "granularity": 65536 + } + ], + "refcount-bits": 16, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "libvirt-5-format", + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "dirty-bitmaps": [ + { + "name": "a", + "recording": true, + "persistent": true, + "busy": false, + "status": "active", + "granularity": 65536, + "count": 0 + } + ], + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.qcow2", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 328192, + "filename": "/tmp/pull4.qcow2", + "format": "file", + "actual-size": 208896, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "libvirt-5-storage", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "/tmp/pull4.qcow2", + "encryption_key_missing": false + } +] diff --git a/tests/qemublocktestdata/bitmap/snapshots.out b/tests/qemublocktestdata/bitmap/snapshots.out new file mode 100644 index 0000000000..9e0bb36fc8 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots.out @@ -0,0 +1,14 @@ +libvirt-1-format: + d: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-2-format: + c: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + d: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-3-format: + a: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + b: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + c: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-4-format: + a: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-5-format: + a: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Add test data gathered from a run of qemu after creating bitmaps and snapshots together in various combinations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + tests/qemublocktestdata/bitmap/snapshots.json | 836 ++++++++++++++++++
Huge!
tests/qemublocktestdata/bitmap/snapshots.out | 14 +
But looks like a good summary of potential configurations.
3 files changed, 851 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out
+ "filename": "/tmp/pull4.1575911550", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 212992, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "bitmaps": [ + {
It's annoying that qemu semi-duplicates information between "bitmaps"...
+ "backing_file": "/tmp/pull4.1575911540", + "dirty-bitmaps": [ + {
and "dirty-bitmaps", but you appear to be grabbing the intended field.
+++ b/tests/qemublocktestdata/bitmap/snapshots.out @@ -0,0 +1,14 @@ +libvirt-1-format: + d: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
More fallout from typo fixes earlier in the series. Reviewed-by: Eric Blake <eblake@redhat.com>
+libvirt-2-format: + c: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + d: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-3-format: + a: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + b: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0 + c: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-4-format: + a: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +libvirt-5-format: + a: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Dec 12, 2019 at 13:38:29 -0600, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Add test data gathered from a run of qemu after creating bitmaps and snapshots together in various combinations.
I'll also add how to get to the bitmap topology.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + tests/qemublocktestdata/bitmap/snapshots.json | 836 ++++++++++++++++++
Huge!
We really want 'query-named-block-nodes' not deal with the backing file. I'll investigate whether there's a simple-enough way to make qemu stop producing the nested input when we provide some new flag.
The object itself has no extra value and it would make testing the code harder. Refactor it to remove just the definition pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 5c9747f09d..1cd466d211 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -174,7 +174,7 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm, static int qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, virJSONValuePtr actions, - virDomainMomentObjPtr *incremental) + virDomainMomentDefPtr *incremental) { g_autoptr(virJSONValue) mergebitmaps = NULL; g_autoptr(virJSONValue) mergebitmapsstore = NULL; @@ -188,7 +188,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, while (*incremental) { if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, dd->domdisk->src->nodeformat, - (*incremental)->def->name) < 0) + (*incremental)->name) < 0) return -1; incremental++; @@ -232,7 +232,7 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm, virDomainBackupDiskDefPtr backupdisk, struct qemuBackupDiskData *dd, virJSONValuePtr actions, - virDomainMomentObjPtr *incremental, + virDomainMomentDefPtr *incremental, virQEMUDriverConfigPtr cfg, bool removeStore) { @@ -323,7 +323,7 @@ qemuBackupDiskPrepareDataOnePull(virJSONValuePtr actions, static ssize_t qemuBackupDiskPrepareData(virDomainObjPtr vm, virDomainBackupDefPtr def, - virDomainMomentObjPtr *incremental, + virDomainMomentDefPtr *incremental, virJSONValuePtr actions, virQEMUDriverConfigPtr cfg, struct qemuBackupDiskData **rdd, @@ -505,24 +505,27 @@ qemuBackupBeginPullExportDisks(virDomainObjPtr vm, * @vm: domain object * @incrFrom: name of checkpoint representing starting point of incremental backup * - * Returns a NULL terminated list of pointers to checkpoints in chronological - * order starting from the 'current' checkpoint until reaching @incrFrom. + * Returns a NULL terminated list of pointers to checkpoint definitions in + * chronological order starting from the 'current' checkpoint until reaching + * @incrFrom. */ -static virDomainMomentObjPtr * +static virDomainMomentDefPtr * qemuBackupBeginCollectIncrementalCheckpoints(virDomainObjPtr vm, const char *incrFrom) { virDomainMomentObjPtr n = virDomainCheckpointGetCurrent(vm->checkpoints); - g_autofree virDomainMomentObjPtr *incr = NULL; + g_autofree virDomainMomentDefPtr *incr = NULL; size_t nincr = 0; while (n) { - if (VIR_APPEND_ELEMENT_COPY(incr, nincr, n) < 0) + virDomainMomentDefPtr def = n->def; + + if (VIR_APPEND_ELEMENT_COPY(incr, nincr, def) < 0) return NULL; - if (STREQ(n->def->name, incrFrom)) { - virDomainMomentObjPtr terminator = NULL; - if (VIR_APPEND_ELEMENT_COPY(incr, nincr, terminator) < 0) + if (STREQ(def->name, incrFrom)) { + def = NULL; + if (VIR_APPEND_ELEMENT_COPY(incr, nincr, def) < 0) return NULL; return g_steal_pointer(&incr); @@ -648,7 +651,7 @@ qemuBackupBegin(virDomainObjPtr vm, bool pull = false; virDomainMomentObjPtr chk = NULL; g_autoptr(virDomainCheckpointDef) chkdef = NULL; - g_autofree virDomainMomentObjPtr *incremental = NULL; + g_autofree virDomainMomentDefPtr *incremental = NULL; g_autoptr(virJSONValue) actions = NULL; struct qemuBackupDiskData *dd = NULL; ssize_t ndd = 0; -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
The object itself has no extra value and it would make testing the code harder. Refactor it to remove just the definition pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Separate the for now incomplete code that collects the bitmaps to be merged for an incremental backup into a separate function. This will allow to add testing prior to the improvement of the algorithm to include snapshots. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 1cd466d211..31949b5399 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -170,30 +170,43 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm, } - -static int -qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, - virJSONValuePtr actions, - virDomainMomentDefPtr *incremental) +static virJSONValuePtr +qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, + virStorageSourcePtr backingChain) { - g_autoptr(virJSONValue) mergebitmaps = NULL; - g_autoptr(virJSONValue) mergebitmapsstore = NULL; + g_autoptr(virJSONValue) ret = NULL; - if (!(mergebitmaps = virJSONValueNewArray())) - return -1; + if (!(ret = virJSONValueNewArray())) + return NULL; /* TODO: this code works only if the bitmaps are present on a single node. * The algorithm needs to be changed so that it looks into the backing chain * so that we can combine all relevant bitmaps for a given backing chain */ while (*incremental) { - if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, - dd->domdisk->src->nodeformat, + if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(ret, + backingChain->nodeformat, (*incremental)->name) < 0) - return -1; + return NULL; incremental++; } + return g_steal_pointer(&ret); +} + + +static int +qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, + virJSONValuePtr actions, + virDomainMomentDefPtr *incremental) +{ + g_autoptr(virJSONValue) mergebitmaps = NULL; + g_autoptr(virJSONValue) mergebitmapsstore = NULL; + + if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental, + dd->domdisk->src))) + return -1; + if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps))) return -1; -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Separate the for now incomplete code that collects the bitmaps to be merged for an incremental backup into a separate function. This will allow to add testing prior to the improvement of the algorithm to
"allow to $VERB" is not idiomatic English; you generally want "allow $SUBJECT to $VERB" or "allow ${VERB}ing". Here, "This will allow the addition of testing prior to ..."
include snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-)
The refactoring is sane. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
The function will require the bitmap topology for the full implementation. To facilitate testing, add the propagation of the necessary data beforehand so that the test code can stay unchanged during the changes.t Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 31949b5399..fac6592366 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -172,7 +172,9 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm, static virJSONValuePtr qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, - virStorageSourcePtr backingChain) + virStorageSourcePtr backingChain, + virHashTablePtr blockNamedNodeData G_GNUC_UNUSED, + const char *diskdst G_GNUC_UNUSED) { g_autoptr(virJSONValue) ret = NULL; @@ -198,13 +200,16 @@ qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, static int qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd, virJSONValuePtr actions, - virDomainMomentDefPtr *incremental) + virDomainMomentDefPtr *incremental, + virHashTablePtr blockNamedNodeData) { g_autoptr(virJSONValue) mergebitmaps = NULL; g_autoptr(virJSONValue) mergebitmapsstore = NULL; if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental, - dd->domdisk->src))) + dd->domdisk->src, + blockNamedNodeData, + dd->domdisk->dst))) return -1; if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps))) @@ -246,6 +251,7 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm, struct qemuBackupDiskData *dd, virJSONValuePtr actions, virDomainMomentDefPtr *incremental, + virHashTablePtr blockNamedNodeData, virQEMUDriverConfigPtr cfg, bool removeStore) { @@ -274,7 +280,8 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm, if (incremental) { dd->incrementalBitmap = g_strdup_printf("backup-%s", dd->domdisk->dst); - if (qemuBackupDiskPrepareOneBitmaps(dd, actions, incremental) < 0) + if (qemuBackupDiskPrepareOneBitmaps(dd, actions, incremental, + blockNamedNodeData) < 0) return -1; } @@ -337,6 +344,7 @@ static ssize_t qemuBackupDiskPrepareData(virDomainObjPtr vm, virDomainBackupDefPtr def, virDomainMomentDefPtr *incremental, + virHashTablePtr blockNamedNodeData, virJSONValuePtr actions, virQEMUDriverConfigPtr cfg, struct qemuBackupDiskData **rdd, @@ -359,7 +367,8 @@ qemuBackupDiskPrepareData(virDomainObjPtr vm, ndisks++; if (qemuBackupDiskPrepareDataOne(vm, backupdisk, dd, actions, - incremental, cfg, removeStore) < 0) + incremental, blockNamedNodeData, + cfg, removeStore) < 0) goto error; if (def->type == VIR_DOMAIN_BACKUP_TYPE_PULL) { @@ -745,8 +754,14 @@ qemuBackupBegin(virDomainObjPtr vm, goto endjob; } - if ((ndd = qemuBackupDiskPrepareData(vm, def, incremental, actions, cfg, &dd, - reuse)) <= 0) { + if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, QEMU_ASYNC_JOB_BACKUP) < 0) + goto endjob; + blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData) + goto endjob; + + if ((ndd = qemuBackupDiskPrepareData(vm, def, incremental, blockNamedNodeData, + actions, cfg, &dd, reuse)) <= 0) { if (ndd == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("no disks selected for backup")); @@ -755,12 +770,6 @@ qemuBackupBegin(virDomainObjPtr vm, goto endjob; } - if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, QEMU_ASYNC_JOB_BACKUP) < 0) - goto endjob; - blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); - if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData) - goto endjob; - if (qemuBackupDiskPrepareStorage(vm, dd, ndd, blockNamedNodeData, reuse) < 0) goto endjob; -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
The function will require the bitmap topology for the full implementation. To facilitate testing, add the propagation of the necessary data beforehand so that the test code can stay unchanged during the changes.t
Stray t.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_backup.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index fac6592366..14cf6bbef0 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -170,7 +170,7 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm, } -static virJSONValuePtr +virJSONValuePtr qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, virStorageSourcePtr backingChain, virHashTablePtr blockNamedNodeData G_GNUC_UNUSED, diff --git a/src/qemu/qemu_backup.h b/src/qemu/qemu_backup.h index 0f76abe067..df67b849be 100644 --- a/src/qemu/qemu_backup.h +++ b/src/qemu/qemu_backup.h @@ -44,3 +44,10 @@ int qemuBackupGetJobInfoStats(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainJobInfoPtr jobInfo); + +/* exported for testing */ +virJSONValuePtr +qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, + virStorageSourcePtr backingChain, + virHashTablePtr blockNamedNodeData, + const char *diskdst); -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_backup.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Add test code which will crawl a fake internal list of checkpoints and generate the list of bitmaps for merging to gather the final bitmap for the backup. The initial tests cover the basic case of all bitmaps being present in the top layer of the backing chain. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 160 ++++++++++++++++++ .../backupmerge/basic-deep-out.json | 22 +++ .../backupmerge/basic-flat-out.json | 6 + .../backupmerge/basic-intermediate-out.json | 10 ++ 4 files changed, 198 insertions(+) create mode 100644 tests/qemublocktestdata/backupmerge/basic-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/basic-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/basic-intermediate-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 25afef46bb..659ce327dc 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -28,6 +28,7 @@ # include "qemu/qemu_block.h" # include "qemu/qemu_qapi.h" # include "qemu/qemu_monitor_json.h" +# include "qemu/qemu_backup.h" # include "qemu/qemu_command.h" @@ -558,6 +559,145 @@ testQemuDetectBitmaps(const void *opaque) } +static virStorageSourcePtr +testQemuBackupIncrementalBitmapCalculateGetFakeImage(size_t idx) +{ + virStorageSourcePtr ret; + + if (!(ret = virStorageSourceNew())) + abort(); + + ret->type = VIR_STORAGE_TYPE_FILE; + ret->format = VIR_STORAGE_FILE_QCOW2; + ret->path = g_strdup_printf("/image%zu", idx); + ret->nodestorage = g_strdup_printf("libvirt-%zu-storage", idx); + ret->nodeformat = g_strdup_printf("libvirt-%zu-format", idx); + + return ret; +} + + +static virStorageSourcePtr +testQemuBackupIncrementalBitmapCalculateGetFakeChain(void) +{ + virStorageSourcePtr ret; + virStorageSourcePtr n; + size_t i; + + n = ret = testQemuBackupIncrementalBitmapCalculateGetFakeImage(1); + + for (i = 2; i < 10; i++) { + n->backingStore = testQemuBackupIncrementalBitmapCalculateGetFakeImage(i); + n = n->backingStore; + } + + return ret; +} + + +typedef virDomainMomentDefPtr testMomentList; + +static void +testMomentListFree(testMomentList *list) +{ + testMomentList *tmp = list; + + if (!list) + return; + + while (*tmp) { + virObjectUnref(*tmp); + tmp++; + } + + g_free(list); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(testMomentList, testMomentListFree); + +static virDomainMomentDefPtr +testQemuBackupGetIncrementalMoment(const char *name) +{ + virDomainCheckpointDefPtr checkpoint = NULL; + + if (!(checkpoint = virDomainCheckpointDefNew())) + abort(); + + checkpoint->parent.name = g_strdup(name); + + return (virDomainMomentDefPtr) checkpoint; +} + + +static virDomainMomentDefPtr * +testQemuBackupGetIncremental(const char *incFrom) +{ + const char *checkpoints[] = {"current", "d", "c", "b", "a"}; + virDomainMomentDefPtr *incr; + size_t i; + + incr = g_new0(virDomainMomentDefPtr, G_N_ELEMENTS(checkpoints) + 1); + + for (i = 0; i < G_N_ELEMENTS(checkpoints); i++) { + incr[i] = testQemuBackupGetIncrementalMoment(checkpoints[i]); + + if (STREQ(incFrom, checkpoints[i])) + break; + } + + return incr; +} + +static const char *backupDataPrefix = "qemublocktestdata/backupmerge/"; + +struct testQemuBackupIncrementalBitmapCalculateData { + const char *name; + virStorageSourcePtr chain; + const char *incremental; + const char *nodedatafile; +}; + + +static int +testQemuBackupIncrementalBitmapCalculate(const void *opaque) +{ + const struct testQemuBackupIncrementalBitmapCalculateData *data = opaque; + g_autoptr(virJSONValue) nodedatajson = NULL; + g_autoptr(virHashTable) nodedata = NULL; + g_autoptr(virJSONValue) mergebitmaps = NULL; + g_autofree char *actual = NULL; + g_autofree char *expectpath = NULL; + g_autoptr(testMomentList) incremental = NULL; + + expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir, + backupDataPrefix, data->name); + + if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, data->nodedatafile, + ".json", NULL))) + return -1; + + if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) { + VIR_TEST_VERBOSE("failed to load nodedata JSON\n"); + return -1; + } + + incremental = testQemuBackupGetIncremental(data->incremental); + + if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental, + data->chain, + nodedata, + "testdisk"))) { + VIR_TEST_VERBOSE("failed to calculate merged bitmaps"); + return -1; + } + + if (!(actual = virJSONValueToString(mergebitmaps, true))) + return -1; + + return virTestCompareToFile(actual, expectpath); +} + + static int mymain(void) { @@ -566,12 +706,16 @@ mymain(void) struct testBackingXMLjsonXMLdata xmljsonxmldata; struct testQemuDiskXMLToJSONData diskxmljsondata; struct testQemuImageCreateData imagecreatedata; + struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata; char *capslatest_x86_64 = NULL; virQEMUCapsPtr caps_x86_64 = NULL; + g_autoptr(virStorageSource) bitmapSourceChain = NULL; if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; + bitmapSourceChain = testQemuBackupIncrementalBitmapCalculateGetFakeChain(); + diskxmljsondata.driver = &driver; imagecreatedata.driver = &driver; @@ -779,6 +923,22 @@ mymain(void) TEST_BITMAP_DETECT("synthetic"); TEST_BITMAP_DETECT("snapshots"); +# define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \ + do { \ + backupbitmapcalcdata.name = testname; \ + backupbitmapcalcdata.chain = source; \ + backupbitmapcalcdata.incremental = incrbackup; \ + backupbitmapcalcdata.nodedatafile = named; \ + if (virTestRun("incremental backup bitmap " testname, \ + testQemuBackupIncrementalBitmapCalculate, \ + &backupbitmapcalcdata) < 0) \ + ret = -1; \ + } while (0) + + TEST_BACKUP_BITMAP_CALCULATE("basic-flat", bitmapSourceChain, "current", "basic"); + TEST_BACKUP_BITMAP_CALCULATE("basic-intermediate", bitmapSourceChain, "d", "basic"); + TEST_BACKUP_BITMAP_CALCULATE("basic-deep", bitmapSourceChain, "a", "basic"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/backupmerge/basic-deep-out.json b/tests/qemublocktestdata/backupmerge/basic-deep-out.json new file mode 100644 index 0000000000..28c3d16259 --- /dev/null +++ b/tests/qemublocktestdata/backupmerge/basic-deep-out.json @@ -0,0 +1,22 @@ +[ + { + "node": "libvirt-1-format", + "name": "current" + }, + { + "node": "libvirt-1-format", + "name": "d" + }, + { + "node": "libvirt-1-format", + "name": "c" + }, + { + "node": "libvirt-1-format", + "name": "b" + }, + { + "node": "libvirt-1-format", + "name": "a" + } +] diff --git a/tests/qemublocktestdata/backupmerge/basic-flat-out.json b/tests/qemublocktestdata/backupmerge/basic-flat-out.json new file mode 100644 index 0000000000..b89252e284 --- /dev/null +++ b/tests/qemublocktestdata/backupmerge/basic-flat-out.json @@ -0,0 +1,6 @@ +[ + { + "node": "libvirt-1-format", + "name": "current" + } +] diff --git a/tests/qemublocktestdata/backupmerge/basic-intermediate-out.json b/tests/qemublocktestdata/backupmerge/basic-intermediate-out.json new file mode 100644 index 0000000000..0dffcafd5f --- /dev/null +++ b/tests/qemublocktestdata/backupmerge/basic-intermediate-out.json @@ -0,0 +1,10 @@ +[ + { + "node": "libvirt-1-format", + "name": "current" + }, + { + "node": "libvirt-1-format", + "name": "d" + } +] -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Add test code which will crawl a fake internal list of checkpoints and generate the list of bitmaps for merging to gather the final bitmap for the backup.
The initial tests cover the basic case of all bitmaps being present in the top layer of the backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
+ +static const char *backupDataPrefix = "qemublocktestdata/backupmerge/"; +
Drop the double space. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
This function looks up a named bitmap for a virStorageSource in the data returned from query-named-block-nodes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 32 ++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 5 +++++ 2 files changed, 37 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index ada2c52947..629a09b897 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2612,3 +2612,35 @@ qemuBlockRemoveImageMetadata(virQEMUDriverPtr driver, return ret; } + + +/** + * qemuBlockNamedNodeDataGetBitmapByName: + * @blockNamedNodeData: hash table returned by qemuMonitorBlockGetNamedNodeData + * @src: disk source to find the bitmap for + * @bitmap: name of the bitmap to find + * + * Looks up a bitmap named @bitmap of the @src image. + */ +qemuBlockNamedNodeDataBitmapPtr +qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData, + virStorageSourcePtr src, + const char *bitmap) +{ + qemuBlockNamedNodeDataPtr nodedata; + size_t i; + + if (!(nodedata = virHashLookup(blockNamedNodeData, src->nodeformat))) + return NULL; + + for (i = 0; i < nodedata->nbitmaps; i++) { + qemuBlockNamedNodeDataBitmapPtr bitmapdata = nodedata->bitmaps[i]; + + if (STRNEQ(bitmapdata->name, bitmap)) + continue; + + return bitmapdata; + } + + return NULL; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 5854641027..1a38e0eccf 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -203,3 +203,8 @@ qemuBlockRemoveImageMetadata(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *diskTarget, virStorageSourcePtr src); + +qemuBlockNamedNodeDataBitmapPtr +qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData, + virStorageSourcePtr src, + const char *bitmap); -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
This function looks up a named bitmap for a virStorageSource in the data returned from query-named-block-nodes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 32 ++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 5 +++++ 2 files changed, 37 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
To allow backups work accross external snapshots we need to improve the algorithm which calculates which bitmaps to merge. The algorithm must look for appropriately named bitmaps in the image and possibly descend into a backing image if the current image does not have the bitmap. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 62 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 14cf6bbef0..294d5999a0 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -173,24 +173,70 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm, virJSONValuePtr qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, virStorageSourcePtr backingChain, - virHashTablePtr blockNamedNodeData G_GNUC_UNUSED, - const char *diskdst G_GNUC_UNUSED) + virHashTablePtr blockNamedNodeData, + const char *diskdst) { + qemuBlockNamedNodeDataBitmapPtr bitmap; g_autoptr(virJSONValue) ret = NULL; + size_t incridx = 0; if (!(ret = virJSONValueNewArray())) return NULL; - /* TODO: this code works only if the bitmaps are present on a single node. - * The algorithm needs to be changed so that it looks into the backing chain - * so that we can combine all relevant bitmaps for a given backing chain */ - while (*incremental) { + if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + backingChain, + incremental[0]->name))) { + virReportError(VIR_ERR_INVALID_ARG, + _("failed to find bitmap '%s' in image '%s%u'"), + incremental[0]->name, diskdst, backingChain->id); + return NULL; + } + + while (bitmap) { + if (bitmap->inconsistent) { + virReportError(VIR_ERR_INVALID_ARG, + _("bitmap '%s' for image '%s%u' is inconsistent"), + bitmap->name, diskdst, backingChain->id); + return NULL; + } + if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(ret, backingChain->nodeformat, - (*incremental)->name) < 0) + bitmap->name) < 0) return NULL; - incremental++; + if (backingChain->backingStore && + (bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + backingChain->backingStore, + incremental[incridx]->name))) { + backingChain = backingChain->backingStore; + continue; + } + + if (incremental[incridx + 1]) { + if ((bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + backingChain, + incremental[incridx + 1]->name))) { + incridx++; + continue; + } + + if (backingChain->backingStore && + (bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + backingChain->backingStore, + incremental[incridx + 1]->name))) { + incridx++; + backingChain = backingChain->backingStore; + continue; + } + + virReportError(VIR_ERR_INVALID_ARG, + _("failed to find bitmap '%s' in image '%s%u'"), + incremental[incridx]->name, diskdst, backingChain->id); + return NULL; + } else { + break; + } } return g_steal_pointer(&ret); -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
To allow backups work accross external snapshots we need to improve the
across
algorithm which calculates which bitmaps to merge.
The algorithm must look for appropriately named bitmaps in the image and possibly descend into a backing image if the current image does not have the bitmap.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 62 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 8 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 4 ++ .../backupmerge/snapshot-deep-out.json | 38 +++++++++++++++++++ .../backupmerge/snapshot-flat-out.json | 6 +++ .../snapshot-intermediate-out.json | 14 +++++++ 4 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 659ce327dc..905a5b6ed2 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -939,6 +939,10 @@ mymain(void) TEST_BACKUP_BITMAP_CALCULATE("basic-intermediate", bitmapSourceChain, "d", "basic"); TEST_BACKUP_BITMAP_CALCULATE("basic-deep", bitmapSourceChain, "a", "basic"); + TEST_BACKUP_BITMAP_CALCULATE("snapshot-flat", bitmapSourceChain, "current", "snapshots"); + TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain, "d", "snapshots"); + TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a", "snapshots"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/backupmerge/snapshot-deep-out.json b/tests/qemublocktestdata/backupmerge/snapshot-deep-out.json new file mode 100644 index 0000000000..526fc8d55b --- /dev/null +++ b/tests/qemublocktestdata/backupmerge/snapshot-deep-out.json @@ -0,0 +1,38 @@ +[ + { + "node": "libvirt-1-format", + "name": "current" + }, + { + "node": "libvirt-1-format", + "name": "d" + }, + { + "node": "libvirt-2-format", + "name": "d" + }, + { + "node": "libvirt-2-format", + "name": "c" + }, + { + "node": "libvirt-3-format", + "name": "c" + }, + { + "node": "libvirt-3-format", + "name": "b" + }, + { + "node": "libvirt-3-format", + "name": "a" + }, + { + "node": "libvirt-4-format", + "name": "a" + }, + { + "node": "libvirt-5-format", + "name": "a" + } +] diff --git a/tests/qemublocktestdata/backupmerge/snapshot-flat-out.json b/tests/qemublocktestdata/backupmerge/snapshot-flat-out.json new file mode 100644 index 0000000000..b89252e284 --- /dev/null +++ b/tests/qemublocktestdata/backupmerge/snapshot-flat-out.json @@ -0,0 +1,6 @@ +[ + { + "node": "libvirt-1-format", + "name": "current" + } +] diff --git a/tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json b/tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json new file mode 100644 index 0000000000..537d776ec6 --- /dev/null +++ b/tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json @@ -0,0 +1,14 @@ +[ + { + "node": "libvirt-1-format", + "name": "current" + }, + { + "node": "libvirt-1-format", + "name": "d" + }, + { + "node": "libvirt-2-format", + "name": "d" + } +] -- 2.23.0
On 12/12/19 11:18 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 4 ++ .../backupmerge/snapshot-deep-out.json | 38 +++++++++++++++++++ .../backupmerge/snapshot-flat-out.json | 6 +++ .../snapshot-intermediate-out.json | 14 +++++++ 4 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (4)
-
Daniel P. Berrangé -
Eric Blake -
Michal Privoznik -
Peter Krempa