Add test infrastructure and a basic test for bitmap deletion. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 59 +++++++++++++++++++ .../checkpointdelete/basic-noparent-out.json | 9 +++ 2 files changed, 68 insertions(+) create mode 100644 tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 3076dc9645..00056a2b90 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -27,6 +27,7 @@ #include "qemu/qemu_qapi.h" #include "qemu/qemu_monitor_json.h" #include "qemu/qemu_backup.h" +#include "qemu/qemu_checkpoint.h" #include "qemu/qemu_command.h" @@ -700,6 +701,50 @@ testQemuBackupIncrementalBitmapCalculate(const void *opaque) } +static const char *checkpointDeletePrefix = "qemublocktestdata/checkpointdelete/"; + +struct testQemuCheckpointDeleteMergeData { + const char *name; + virStorageSourcePtr chain; + const char *deletebitmap; + const char *parentbitmap; +}; + + +static int +testQemuCheckpointDeleteMerge(const void *opaque) +{ + const struct testQemuCheckpointDeleteMergeData *data = opaque; + g_autofree char *actual = NULL; + g_autofree char *expectpath = NULL; + g_autoptr(virJSONValue) actions = NULL; + bool currentcheckpoint; + + expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir, + checkpointDeletePrefix, data->name); + + if (!(actions = virJSONValueNewArray())) + return -1; + + /* hack to get the 'current' state until the function stops accepting it */ + currentcheckpoint = STREQ("current", data->deletebitmap); + + if (qemuCheckpointDiscardDiskBitmaps(data->chain, + data->deletebitmap, + data->parentbitmap, + currentcheckpoint, + actions) < 0) { + VIR_TEST_VERBOSE("failed to generate checkpoint delete transaction\n"); + return -1; + } + + if (!(actual = virJSONValueToString(actions, true))) + return -1; + + return virTestCompareToFile(actual, expectpath); +} + + static int mymain(void) { @@ -709,6 +754,7 @@ mymain(void) struct testQemuDiskXMLToJSONData diskxmljsondata; struct testQemuImageCreateData imagecreatedata; struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata; + struct testQemuCheckpointDeleteMergeData checkpointdeletedata; char *capslatest_x86_64 = NULL; virQEMUCapsPtr caps_x86_64 = NULL; g_autoptr(virStorageSource) bitmapSourceChain = NULL; @@ -945,6 +991,19 @@ mymain(void) TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain, "d", "snapshots"); TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a", "snapshots"); +#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp) \ + do { \ + checkpointdeletedata.name = testname; \ + checkpointdeletedata.chain = bitmapSourceChain; \ + checkpointdeletedata.deletebitmap = delbmp; \ + checkpointdeletedata.parentbitmap = parbmp; \ + if (virTestRun("checkpoint delete " testname, \ + testQemuCheckpointDeleteMerge, &checkpointdeletedata) < 0) \ + ret = -1; \ + } while (0) + + TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json b/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json new file mode 100644 index 0000000000..e87382fdb4 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json @@ -0,0 +1,9 @@ +[ + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "a" + } + } +] -- 2.24.1

Add all intermediate steps and deletion of the current checkpoint on a flat (single-image) disk image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 4 +++ .../checkpointdelete/basic-current-out.json | 29 +++++++++++++++++++ .../basic-intermediate1-out.json | 22 ++++++++++++++ .../basic-intermediate2-out.json | 22 ++++++++++++++ .../basic-intermediate3-out.json | 22 ++++++++++++++ 5 files changed, 99 insertions(+) create mode 100644 tests/qemublocktestdata/checkpointdelete/basic-current-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 00056a2b90..e062f1a4cb 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1003,6 +1003,10 @@ mymain(void) } while (0) TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL); + TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a"); + TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b"); + TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c"); + TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/checkpointdelete/basic-current-out.json b/tests/qemublocktestdata/checkpointdelete/basic-current-out.json new file mode 100644 index 0000000000..1b607567e8 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/basic-current-out.json @@ -0,0 +1,29 @@ +[ + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-1-format", + "name": "d" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "current" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json b/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json new file mode 100644 index 0000000000..eccb7ed15f --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json @@ -0,0 +1,22 @@ +[ + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "a", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "b" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "b" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json b/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json new file mode 100644 index 0000000000..de40e4b5b0 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json @@ -0,0 +1,22 @@ +[ + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "c" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json b/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json new file mode 100644 index 0000000000..b5d85f43f0 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json @@ -0,0 +1,22 @@ +[ + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "d" + } + } +] -- 2.24.1

Add a faked qemu output which would simulate scenario where libvirt would take a snapshot and checkpoint simultaneously. This is visible in libvirt-2-format node where bitmap 'c' appears, but bitmap 'b' which is active in the previous layer is not present. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + .../snapshots-synthetic-checkpoint.json | 827 ++++++++++++++++++ .../bitmap/snapshots-synthetic-checkpoint.out | 13 + 3 files changed, 841 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index e062f1a4cb..861b4e86ae 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -970,6 +970,7 @@ mymain(void) TEST_BITMAP_DETECT("basic"); TEST_BITMAP_DETECT("synthetic"); TEST_BITMAP_DETECT("snapshots"); + TEST_BITMAP_DETECT("snapshots-synthetic-checkpoint"); #define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \ do { \ diff --git a/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json new file mode 100644 index 0000000000..25cc150d67 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json @@ -0,0 +1,827 @@ +[ + { + "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": true, + "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.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-synthetic-checkpoint.out b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out new file mode 100644 index 0000000000..0270657001 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out @@ -0,0 +1,13 @@ +libvirt-1-format: + d: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 + current: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 +libvirt-2-format: + c: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 + d: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 +libvirt-3-format: + a: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 + b: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 +libvirt-4-format: + a: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 +libvirt-5-format: + a: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 -- 2.24.1

Allow deleting of checkpoints when snapshots were created along. The code tracks and modifies the checkpoint list so that backups can still be taken with such a backing chain. This unfortunately requires to rename few bitmaps (by copying and deleting them) in some cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 112 ++++++++++++++++++++++++++++--------- src/qemu/qemu_checkpoint.h | 5 +- tests/qemublocktest.c | 34 +++++++---- 3 files changed, 111 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index e75cdd0458..087a740cf8 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -24,6 +24,7 @@ #include "qemu_capabilities.h" #include "qemu_monitor.h" #include "qemu_domain.h" +#include "qemu_block.h" #include "virerror.h" #include "virlog.h" @@ -150,39 +151,92 @@ qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm, int qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, + virHashTablePtr blockNamedNodeData, const char *delbitmap, const char *parentbitmap, - bool chkcurrent, - virJSONValuePtr actions) + virJSONValuePtr actions, + const char *diskdst) { - if (parentbitmap) { - g_autoptr(virJSONValue) arr = NULL; + virStorageSourcePtr n = src; - if (!(arr = virJSONValueNewArray())) - return -1; + /* find the backing chain entry with bitmap named '@bitmap' */ + while (n) { + qemuBlockNamedNodeDataBitmapPtr tmp; - if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, - src->nodeformat, - delbitmap) < 0) - return -1; + if ((tmp = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + n, delbitmap))) { + break; + } + + n = n->backingStore; + } + + if (!n) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("bitmap '%s' not found in backing chain of '%s'"), + delbitmap, diskdst); + return -1; + } - if (chkcurrent) { - if (qemuMonitorTransactionBitmapEnable(actions, - src->nodeformat, - parentbitmap) < 0) + while (n) { + qemuBlockNamedNodeDataBitmapPtr srcbitmap; + + if (!(srcbitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + n, delbitmap))) + break; + + /* For the actual checkpoint deletion we will merge any bitmap into the + * bitmap of the parent checkpoint (@mergebitmap) or for any image + * where the parent checkpoint bitmap is not present we must rename + * the bitmap of the deleted checkpoint into the bitmap of the parent + * checkpoint as qemu can't currently take the allocation map and turn + * it into a bitmap and thus we wouldn't be able to do a backup. */ + if (parentbitmap) { + qemuBlockNamedNodeDataBitmapPtr dstbitmap; + g_autoptr(virJSONValue) arr = NULL; + + dstbitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, + n, parentbitmap); + + if (dstbitmap) { + if (srcbitmap->recording && !dstbitmap->recording) { + if (qemuMonitorTransactionBitmapEnable(actions, + n->nodeformat, + dstbitmap->name) < 0) + return -1; + } + + } else { + if (qemuMonitorTransactionBitmapAdd(actions, + n->nodeformat, + parentbitmap, + true, + !srcbitmap->recording, + srcbitmap->granularity) < 0) + return -1; + } + + if (!(arr = virJSONValueNewArray())) + return -1; + + if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, + n->nodeformat, + srcbitmap->name) < 0) + return -1; + + if (qemuMonitorTransactionBitmapMerge(actions, + n->nodeformat, + parentbitmap, &arr) < 0) return -1; } - if (qemuMonitorTransactionBitmapMerge(actions, - src->nodeformat, - parentbitmap, &arr) < 0) + if (qemuMonitorTransactionBitmapRemove(actions, + n->nodeformat, + srcbitmap->name) < 0) return -1; - } - if (qemuMonitorTransactionBitmapRemove(actions, - src->nodeformat, - delbitmap) < 0) - return -1; + n = n->backingStore; + } return 0; } @@ -191,11 +245,11 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, static int qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, virDomainCheckpointDefPtr chkdef, - bool chkcurrent, virDomainMomentObjPtr parent) { qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverPtr driver = priv->driver; + g_autoptr(virHashTable) blockNamedNodeData = NULL; int rc; g_autoptr(virJSONValue) actions = NULL; size_t i; @@ -203,6 +257,11 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, if (!(actions = virJSONValueNewArray())) return -1; + qemuDomainObjEnterMonitor(driver, vm); + blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); + if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData) + return -1; + for (i = 0; i < chkdef->ndisks; i++) { virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i]; virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, chkdisk->name); @@ -223,8 +282,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, chkdisk->name))) parentbitmap = parentchkdisk->name; - if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, chkdisk->bitmap, - parentbitmap, chkcurrent, actions) < 0) + if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData, + chkdisk->bitmap, parentbitmap, + actions, domdisk->dst) < 0) return -1; } @@ -262,7 +322,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver, virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk); parent = virDomainCheckpointFindByName(vm->checkpoints, chk->def->parent_name); - if (qemuCheckpointDiscardBitmaps(vm, chkdef, chkcurrent, parent) < 0) + if (qemuCheckpointDiscardBitmaps(vm, chkdef, parent) < 0) return -1; } diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index 85fd453d50..976b1eed0f 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -74,7 +74,8 @@ qemuCheckpointRollbackMetadata(virDomainObjPtr vm, int qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, + virHashTablePtr blockNamedNodeData, const char *delbitmap, const char *parentbitmap, - bool chkcurrent, - virJSONValuePtr actions); + virJSONValuePtr actions, + const char *diskdst); diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 861b4e86ae..6194c45ec8 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -708,6 +708,7 @@ struct testQemuCheckpointDeleteMergeData { virStorageSourcePtr chain; const char *deletebitmap; const char *parentbitmap; + const char *nodedatafile; }; @@ -718,22 +719,30 @@ testQemuCheckpointDeleteMerge(const void *opaque) g_autofree char *actual = NULL; g_autofree char *expectpath = NULL; g_autoptr(virJSONValue) actions = NULL; - bool currentcheckpoint; + g_autoptr(virJSONValue) nodedatajson = NULL; + g_autoptr(virHashTable) nodedata = NULL; expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir, checkpointDeletePrefix, data->name); - if (!(actions = virJSONValueNewArray())) + if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, data->nodedatafile, + ".json", NULL))) return -1; - /* hack to get the 'current' state until the function stops accepting it */ - currentcheckpoint = STREQ("current", data->deletebitmap); + if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) { + VIR_TEST_VERBOSE("failed to load nodedata JSON\n"); + return -1; + } + + if (!(actions = virJSONValueNewArray())) + return -1; if (qemuCheckpointDiscardDiskBitmaps(data->chain, + nodedata, data->deletebitmap, data->parentbitmap, - currentcheckpoint, - actions) < 0) { + actions, + "testdisk") < 0) { VIR_TEST_VERBOSE("failed to generate checkpoint delete transaction\n"); return -1; } @@ -992,22 +1001,23 @@ mymain(void) TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain, "d", "snapshots"); TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a", "snapshots"); -#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp) \ +#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp, named) \ do { \ checkpointdeletedata.name = testname; \ checkpointdeletedata.chain = bitmapSourceChain; \ checkpointdeletedata.deletebitmap = delbmp; \ checkpointdeletedata.parentbitmap = parbmp; \ + checkpointdeletedata.nodedatafile = named; \ if (virTestRun("checkpoint delete " testname, \ testQemuCheckpointDeleteMerge, &checkpointdeletedata) < 0) \ ret = -1; \ } while (0) - TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL); - TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a"); - TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b"); - TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c"); - TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d"); + TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL, "basic"); + TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a", "basic"); + TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b", "basic"); + TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c", "basic"); + TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d", "basic"); cleanup: virHashFree(diskxmljsondata.schema); -- 2.24.1

Add test cases for merging various pairs of bitmaps when snapshots were created together with checkpoints. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 6 ++ .../snapshots-current-out.json | 29 +++++++++ .../snapshots-intermediate1-out.json | 22 +++++++ .../snapshots-intermediate2-out.json | 59 +++++++++++++++++++ .../snapshots-intermediate3-out.json | 59 +++++++++++++++++++ .../snapshots-noparent-out.json | 23 ++++++++ 6 files changed, 198 insertions(+) create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 6194c45ec8..1c04929e81 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1019,6 +1019,12 @@ mymain(void) TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c", "basic"); TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d", "basic"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-noparent", "a", NULL, "snapshots"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate1", "b", "a", "snapshots"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate2", "c", "b", "snapshots"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate3", "d", "c", "snapshots"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-current", "current", "d", "snapshots"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json new file mode 100644 index 0000000000..1b607567e8 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json @@ -0,0 +1,29 @@ +[ + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-1-format", + "name": "d" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "current" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json new file mode 100644 index 0000000000..29fefeea63 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json @@ -0,0 +1,22 @@ +[ + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-3-format", + "target": "a", + "bitmaps": [ + { + "node": "libvirt-3-format", + "name": "b" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-3-format", + "name": "b" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json new file mode 100644 index 0000000000..4da21a9df7 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json @@ -0,0 +1,59 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "libvirt-2-format", + "name": "b", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-2-format", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-2-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-2-format", + "name": "c" + } + }, + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-3-format", + "name": "b" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-3-format", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-3-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-3-format", + "name": "c" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json new file mode 100644 index 0000000000..dc87dd60b8 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json @@ -0,0 +1,59 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "libvirt-1-format", + "name": "c", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "d" + } + }, + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-2-format", + "name": "c" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-2-format", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-2-format", + "name": "d" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-2-format", + "name": "d" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json new file mode 100644 index 0000000000..45a84b47c2 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json @@ -0,0 +1,23 @@ +[ + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-3-format", + "name": "a" + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-4-format", + "name": "a" + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-5-format", + "name": "a" + } + } +] -- 2.24.1

Use the synthetic test data to verify that the algorithm correctly picks bitmaps to merge when the bitmap is changed along with the image itself. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 7 +++ ...hots-synthetic-checkpoint-current-out.json | 29 +++++++++ ...ynthetic-checkpoint-intermediate1-out.json | 29 +++++++++ ...ynthetic-checkpoint-intermediate2-out.json | 32 ++++++++++ ...ynthetic-checkpoint-intermediate3-out.json | 59 +++++++++++++++++++ ...ots-synthetic-checkpoint-noparent-out.json | 23 ++++++++ 6 files changed, 179 insertions(+) create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json create mode 100644 tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 1c04929e81..306f6310ee 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1025,6 +1025,13 @@ mymain(void) TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate3", "d", "c", "snapshots"); TEST_CHECKPOINT_DELETE_MERGE("snapshots-current", "current", "d", "snapshots"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-noparent", "a", NULL, "snapshots-synthetic-checkpoint"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate1", "b", "a", "snapshots-synthetic-checkpoint"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate2", "c", "b", "snapshots-synthetic-checkpoint"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate3", "d", "c", "snapshots-synthetic-checkpoint"); + TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-current", "current", "d", "snapshots-synthetic-checkpoint"); + + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json new file mode 100644 index 0000000000..1b607567e8 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json @@ -0,0 +1,29 @@ +[ + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-1-format", + "name": "d" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "current" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json new file mode 100644 index 0000000000..e979691e6f --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json @@ -0,0 +1,29 @@ +[ + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-3-format", + "name": "a" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-3-format", + "target": "a", + "bitmaps": [ + { + "node": "libvirt-3-format", + "name": "b" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-3-format", + "name": "b" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json new file mode 100644 index 0000000000..e82098918a --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json @@ -0,0 +1,32 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "libvirt-2-format", + "name": "b", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-2-format", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-2-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-2-format", + "name": "c" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json new file mode 100644 index 0000000000..dc87dd60b8 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json @@ -0,0 +1,59 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "libvirt-1-format", + "name": "c", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-1-format", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-1-format", + "name": "d" + } + }, + { + "type": "block-dirty-bitmap-enable", + "data": { + "node": "libvirt-2-format", + "name": "c" + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "libvirt-2-format", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-2-format", + "name": "d" + } + ] + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-2-format", + "name": "d" + } + } +] diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json new file mode 100644 index 0000000000..45a84b47c2 --- /dev/null +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json @@ -0,0 +1,23 @@ +[ + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-3-format", + "name": "a" + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-4-format", + "name": "a" + } + }, + { + "type": "block-dirty-bitmap-remove", + "data": { + "node": "libvirt-5-format", + "name": "a" + } + } +] -- 2.24.1

Allow qemu access to modify backing files in case when we want to delete a checkpoint. This patch adds tracking of which images need to be relabelled when calculating the transaction, the code to relabel them and rollback. To verify that stuff works we also output the list of images to relabel into the test case output files in qemublocktest. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 35 ++++++++++++++++--- src/qemu/qemu_checkpoint.h | 3 +- tests/qemublocktest.c | 19 ++++++++-- .../snapshots-intermediate1-out.json | 2 ++ .../snapshots-intermediate2-out.json | 3 ++ .../snapshots-intermediate3-out.json | 2 ++ .../snapshots-noparent-out.json | 4 +++ ...ynthetic-checkpoint-intermediate1-out.json | 2 ++ ...ynthetic-checkpoint-intermediate2-out.json | 2 ++ ...ynthetic-checkpoint-intermediate3-out.json | 2 ++ ...ots-synthetic-checkpoint-noparent-out.json | 4 +++ 11 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 087a740cf8..6ef35ea1a1 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -155,7 +155,8 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, const char *delbitmap, const char *parentbitmap, virJSONValuePtr actions, - const char *diskdst) + const char *diskdst, + GSList **reopenimages) { virStorageSourcePtr n = src; @@ -235,6 +236,9 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, srcbitmap->name) < 0) return -1; + if (n != src) + *reopenimages = g_slist_prepend(*reopenimages, n); + n = n->backingStore; } @@ -250,9 +254,12 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverPtr driver = priv->driver; g_autoptr(virHashTable) blockNamedNodeData = NULL; - int rc; + int rc = -1; g_autoptr(virJSONValue) actions = NULL; size_t i; + g_autoptr(GSList) reopenimages = NULL; + g_autoptr(GSList) relabelimages = NULL; + GSList *next; if (!(actions = virJSONValueNewArray())) return -1; @@ -284,16 +291,34 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData, chkdisk->bitmap, parentbitmap, - actions, domdisk->dst) < 0) + actions, domdisk->dst, + &reopenimages) < 0) return -1; } + /* label any non-top images for read-write access */ + for (next = reopenimages; next; next = next->next) { + virStorageSourcePtr src = next->data; + + if (qemuDomainStorageSourceAccessAllow(driver, vm, src, false, false) < 0) + goto relabel; + + relabelimages = g_slist_prepend(relabelimages, src); + } + qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorTransaction(priv->mon, &actions); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; - return 0; + relabel: + for (next = relabelimages; next; next = next->next) { + virStorageSourcePtr src = next->data; + + ignore_value(qemuDomainStorageSourceAccessAllow(driver, vm, src, true, false)); + } + + return rc; } diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index 976b1eed0f..cf1e9e46cb 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -78,4 +78,5 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, const char *delbitmap, const char *parentbitmap, virJSONValuePtr actions, - const char *diskdst); + const char *diskdst, + GSList **reopenimages); diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 306f6310ee..50dcb86810 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -721,6 +721,9 @@ testQemuCheckpointDeleteMerge(const void *opaque) g_autoptr(virJSONValue) actions = NULL; g_autoptr(virJSONValue) nodedatajson = NULL; g_autoptr(virHashTable) nodedata = NULL; + g_autoptr(GSList) reopenimages = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + GSList *tmp; expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir, checkpointDeletePrefix, data->name); @@ -742,14 +745,26 @@ testQemuCheckpointDeleteMerge(const void *opaque) data->deletebitmap, data->parentbitmap, actions, - "testdisk") < 0) { + "testdisk", + &reopenimages) < 0) { VIR_TEST_VERBOSE("failed to generate checkpoint delete transaction\n"); return -1; } - if (!(actual = virJSONValueToString(actions, true))) + if (virJSONValueToBuffer(actions, &buf, true) < 0) return -1; + if (reopenimages) { + virBufferAddLit(&buf, "reopen nodes:\n"); + + for (tmp = reopenimages; tmp; tmp = tmp->next) { + virStorageSourcePtr src = tmp->data; + virBufferAsprintf(&buf, "%s\n", src->nodeformat); + } + } + + actual = virBufferContentAndReset(&buf); + return virTestCompareToFile(actual, expectpath); } diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json index 29fefeea63..c9bda3a17a 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json @@ -20,3 +20,5 @@ } } ] +reopen nodes: +libvirt-3-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json index 4da21a9df7..8a0e3f2cff 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json @@ -57,3 +57,6 @@ } } ] +reopen nodes: +libvirt-3-format +libvirt-2-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json index dc87dd60b8..211bc40baf 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json @@ -57,3 +57,5 @@ } } ] +reopen nodes: +libvirt-2-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json index 45a84b47c2..f750f44da2 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json @@ -21,3 +21,7 @@ } } ] +reopen nodes: +libvirt-5-format +libvirt-4-format +libvirt-3-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json index e979691e6f..d7e6d18637 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json @@ -27,3 +27,5 @@ } } ] +reopen nodes: +libvirt-3-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json index e82098918a..cfbff010c2 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json @@ -30,3 +30,5 @@ } } ] +reopen nodes: +libvirt-2-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json index dc87dd60b8..211bc40baf 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json @@ -57,3 +57,5 @@ } } ] +reopen nodes: +libvirt-2-format diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json index 45a84b47c2..f750f44da2 100644 --- a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json +++ b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json @@ -21,3 +21,7 @@ } } ] +reopen nodes: +libvirt-5-format +libvirt-4-format +libvirt-3-format -- 2.24.1

Create a wrapper for qemuBlockGetNamedNodeData named qemuBlockGetNamedNodeData. The purpose of the wrapper is to integrate the monitor handling functionality and in the future possible qemuCaps-based flags. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 5 +---- src/qemu/qemu_block.c | 20 ++++++++++++++++++++ src/qemu/qemu_block.h | 4 ++++ src/qemu/qemu_driver.c | 16 ++++------------ 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 23518a5d40..8b1e9a7e19 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -830,10 +830,7 @@ 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) + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_BACKUP))) goto endjob; if ((ndd = qemuBackupDiskPrepareData(vm, def, incremental, blockNamedNodeData, diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 22f03da485..13e240fdac 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2670,3 +2670,23 @@ qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData, return NULL; } + + +virHashTablePtr +qemuBlockGetNamedNodeData(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverPtr driver = priv->driver; + g_autoptr(virHashTable) blockNamedNodeData = NULL; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return NULL; + + blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData) + return NULL; + + return g_steal_pointer(&blockNamedNodeData); +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 1a38e0eccf..68646cbf2e 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -208,3 +208,7 @@ qemuBlockNamedNodeDataBitmapPtr qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData, virStorageSourcePtr src, const char *bitmap); + +virHashTablePtr +qemuBlockGetNamedNodeData(virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8bb845298b..47f0754a1a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15642,15 +15642,9 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (!(actions = virJSONValueNewArray())) return -1; - if (blockdev) { - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - return -1; - - blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); - - if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData) - return -1; - } + if (blockdev && + !(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob))) + return -1; /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ @@ -18354,9 +18348,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, priv->qemuCaps))) goto endjob; } else { - qemuDomainObjEnterMonitor(driver, vm); - blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockNamedNodeData) + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) goto endjob; if (qemuBlockStorageSourceCreateDetectSize(blockNamedNodeData, -- 2.24.1

Add a helper that concatenates the second array into the first. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virjson.c | 31 +++++++++++++++++++++++++++++++ src/util/virjson.h | 2 ++ 3 files changed, 34 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ebf830791e..b2d2420415 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2284,6 +2284,7 @@ virISCSIScanTargets; virJSONStringReformat; virJSONValueArrayAppend; virJSONValueArrayAppendString; +virJSONValueArrayConcat; virJSONValueArrayForeachSteal; virJSONValueArrayGet; virJSONValueArraySize; diff --git a/src/util/virjson.c b/src/util/virjson.c index 988a09e956..50993648eb 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -811,6 +811,37 @@ virJSONValueArrayAppendString(virJSONValuePtr object, } +/** + * virJSONValueArrayConcat: + * @a: JSON value array (destination) + * @c: JSON value array (source) + * + * Merges the members of @c array into @a. The values are stolen from @c. + */ +int +virJSONValueArrayConcat(virJSONValuePtr a, + virJSONValuePtr c) +{ + size_t i; + + if (a->type != VIR_JSON_TYPE_ARRAY || + c->type != VIR_JSON_TYPE_ARRAY) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("expecting JSON array")); + return -1; + } + + a->data.array.values = g_renew(virJSONValuePtr, a->data.array.values, + a->data.array.nvalues + c->data.array.nvalues); + + for (i = 0; i < c->data.array.nvalues; i++) + a->data.array.values[a->data.array.nvalues++] = g_steal_pointer(&c->data.array.values[i]); + + c->data.array.nvalues = 0; + + return 0; +} + + int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key) diff --git a/src/util/virjson.h b/src/util/virjson.h index 7a6b063b17..0894e91b59 100644 --- a/src/util/virjson.h +++ b/src/util/virjson.h @@ -71,6 +71,8 @@ virJSONValuePtr virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap); int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONValuePtr value); int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value); +int virJSONValueArrayConcat(virJSONValuePtr a, + virJSONValuePtr c); int virJSONValueObjectHasKey(virJSONValuePtr object, const char *key); virJSONValuePtr virJSONValueObjectGet(virJSONValuePtr object, const char *key); -- 2.24.1

Use the glib allocation function that never returns NULL and remove the now dead-code checks from all callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/locking/lock_daemon.c | 4 ++-- src/logging/log_handler.c | 3 +-- src/network/leaseshelper.c | 6 +----- src/qemu/qemu_agent.c | 6 +----- src/qemu/qemu_backup.c | 6 ++---- src/qemu/qemu_block.c | 9 +++------ src/qemu/qemu_blockjob.c | 3 +-- src/qemu/qemu_checkpoint.c | 9 +++------ src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_firmware.c | 12 ++++-------- src/qemu/qemu_migration_params.c | 3 +-- src/qemu/qemu_monitor_json.c | 3 +-- src/rpc/virnetserver.c | 6 ++---- src/rpc/virnetserverservice.c | 3 +-- src/util/virjson.c | 13 ++----------- src/util/virlockspace.c | 6 ++---- src/util/virmacmap.c | 8 ++++---- tests/qemublocktest.c | 3 +-- tests/qemumonitorjsontest.c | 5 ++--- 19 files changed, 35 insertions(+), 76 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 7ea228ce37..fd376fef2a 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -949,8 +949,8 @@ virLockDaemonPreExecRestart(const char *state_file, goto cleanup; } - if (!(lockspaces = virJSONValueNewArray())) - goto cleanup; + lockspaces = virJSONValueNewArray(); + if (virJSONValueObjectAppend(object, "lockspaces", lockspaces) < 0) { virJSONValueFree(lockspaces); goto cleanup; diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 030c9d66e3..973c52c7cd 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -619,8 +619,7 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler) if (!ret) return NULL; - if (!(files = virJSONValueNewArray())) - goto error; + files = virJSONValueNewArray(); if (virJSONValueObjectAppend(ret, "files", files) < 0) { virJSONValueFree(files); diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index f1a061066e..dd1d5f70ee 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -200,11 +200,7 @@ main(int argc, char **argv) break; } - if (!(leases_array_new = virJSONValueNewArray())) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); - goto cleanup; - } + leases_array_new = virJSONValueNewArray(); if (virLeaseReadCustomLeaseFile(leases_array_new, custom_lease_file, delete ? ip : NULL, &server_duid) < 0) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 4374235d34..7d01d21a11 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1201,9 +1201,6 @@ qemuAgentMakeStringsArray(const char **strings, unsigned int len) size_t i; virJSONValuePtr ret = virJSONValueNewArray(), str; - if (!ret) - return NULL; - for (i = 0; i < len; i++) { str = virJSONValueNewString(strings[i]); if (!str) @@ -1534,8 +1531,7 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr mon, *nmodified = 0; /* create the key data array */ - if (!(cpus = virJSONValueNewArray())) - goto cleanup; + cpus = virJSONValueNewArray(); for (i = 0; i < ninfo; i++) { qemuAgentCPUInfoPtr in = &info[i]; diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 8b1e9a7e19..2cc6ff7a42 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -180,8 +180,7 @@ qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, g_autoptr(virJSONValue) ret = NULL; size_t incridx = 0; - if (!(ret = virJSONValueNewArray())) - return NULL; + ret = virJSONValueNewArray(); if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, backingChain, @@ -819,8 +818,7 @@ qemuBackupBegin(virDomainObjPtr vm, !(incremental = qemuBackupBeginCollectIncrementalCheckpoints(vm, def->incremental))) goto endjob; - if (!(actions = virJSONValueNewArray())) - goto endjob; + actions = virJSONValueNewArray(); /* The 'chk' checkpoint must be rolled back if the transaction command * which creates it on disk is not executed or fails */ diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 13e240fdac..03f029368e 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -524,8 +524,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, virStorageNetHostDefPtr host; size_t i; - if (!(servers = virJSONValueNewArray())) - return NULL; + servers = virJSONValueNewArray(); for (i = 0; i < src->nhosts; i++) { host = src->hosts + i; @@ -590,8 +589,7 @@ qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) virStorageNetHostDefPtr host; size_t i; - if (!(servers = virJSONValueNewArray())) - return NULL; + servers = virJSONValueNewArray(); for (i = 0; i < src->nhosts; i++) { host = src->hosts + i; @@ -837,8 +835,7 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src, username = srcPriv->secinfo->s.aes.username; keysecret = srcPriv->secinfo->s.aes.alias; /* the auth modes are modelled after our old command line generator */ - if (!(authmodes = virJSONValueNewArray())) - return NULL; + authmodes = virJSONValueNewArray(); if (!(mode = virJSONValueNewString("cephx")) || virJSONValueArrayAppend(authmodes, mode) < 0) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index e04fcf69d1..3dc9222a6f 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1344,8 +1344,7 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, return; if (job->data.backup.bitmap) { - if (!(actions = virJSONValueNewArray())) - return; + actions = virJSONValueNewArray(); if (qemuMonitorTransactionBitmapRemove(actions, job->disk->src->nodeformat, diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 6ef35ea1a1..a60fbcdbac 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -217,8 +217,7 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src, return -1; } - if (!(arr = virJSONValueNewArray())) - return -1; + arr = virJSONValueNewArray(); if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, n->nodeformat, @@ -261,8 +260,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, g_autoptr(GSList) relabelimages = NULL; GSList *next; - if (!(actions = virJSONValueNewArray())) - return -1; + actions = virJSONValueNewArray(); qemuDomainObjEnterMonitor(driver, vm); blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon); @@ -535,8 +533,7 @@ qemuCheckpointCreateCommon(virQEMUDriverPtr driver, if ((parent = virDomainCheckpointGetCurrent(vm->checkpoints))) (*def)->parent.parent_name = g_strdup(parent->def->name); - if (!(tmpactions = virJSONValueNewArray())) - return -1; + tmpactions = virJSONValueNewArray(); if (qemuCheckpointAddActions(vm, tmpactions, parent, *def) < 0) return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 47f0754a1a..021e5a2732 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15639,8 +15639,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (virDomainObjCheckActive(vm) < 0) return -1; - if (!(actions = virJSONValueNewArray())) - return -1; + actions = virJSONValueNewArray(); if (blockdev && !(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob))) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 7fb57913e8..68e2c6b40f 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -651,8 +651,7 @@ qemuFirmwareInterfaceFormat(virJSONValuePtr doc, g_autoptr(virJSONValue) interfacesJSON = NULL; size_t i; - if (!(interfacesJSON = virJSONValueNewArray())) - return -1; + interfacesJSON = virJSONValueNewArray(); for (i = 0; i < fw->ninterfaces; i++) { if (virJSONValueArrayAppendString(interfacesJSON, @@ -799,8 +798,7 @@ qemuFirmwareTargetFormat(virJSONValuePtr doc, g_autoptr(virJSONValue) targetsJSON = NULL; size_t i; - if (!(targetsJSON = virJSONValueNewArray())) - return -1; + targetsJSON = virJSONValueNewArray(); for (i = 0; i < fw->ntargets; i++) { qemuFirmwareTargetPtr t = fw->targets[i]; @@ -816,8 +814,7 @@ qemuFirmwareTargetFormat(virJSONValuePtr doc, virQEMUCapsArchToString(t->architecture)) < 0) return -1; - if (!(machines = virJSONValueNewArray())) - return -1; + machines = virJSONValueNewArray(); for (j = 0; j < t->nmachines; j++) { if (virJSONValueArrayAppendString(machines, @@ -851,8 +848,7 @@ qemuFirmwareFeatureFormat(virJSONValuePtr doc, g_autoptr(virJSONValue) featuresJSON = NULL; size_t i; - if (!(featuresJSON = virJSONValueNewArray())) - return -1; + featuresJSON = virJSONValueNewArray(); for (i = 0; i < fw->nfeatures; i++) { if (virJSONValueArrayAppendString(featuresJSON, diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 1b28e5e031..dd1ad9349b 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -785,8 +785,7 @@ qemuMigrationCapsToJSON(virBitmapPtr caps, qemuMigrationCapability bit; const char *name; - if (!(json = virJSONValueNewArray())) - return NULL; + json = virJSONValueNewArray(); for (bit = 0; bit < QEMU_MIGRATION_CAP_LAST; bit++) { bool supported = false; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 981d091ba0..dc1fc310ca 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4800,8 +4800,7 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, size_t i; /* create the key data array */ - if (!(keys = virJSONValueNewArray())) - goto cleanup; + keys = virJSONValueNewArray(); for (i = 0; i < nkeycodes; i++) { if (keycodes[i] > 0xffff) { diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 4122636805..c87dade1a8 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -603,8 +603,7 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) goto error; } - if (!(services = virJSONValueNewArray())) - goto error; + services = virJSONValueNewArray(); if (virJSONValueObjectAppend(object, "services", services) < 0) { virJSONValueFree(services); @@ -622,8 +621,7 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) } } - if (!(clients = virJSONValueNewArray())) - goto error; + clients = virJSONValueNewArray(); if (virJSONValueObjectAppend(object, "clients", clients) < 0) { virJSONValueFree(clients); diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 5d1178f899..0a003e5814 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -353,8 +353,7 @@ virJSONValuePtr virNetServerServicePreExecRestart(virNetServerServicePtr svc) if (virJSONValueObjectAppendNumberUint(object, "nrequests_client_max", svc->nrequests_client_max) < 0) goto error; - if (!(socks = virJSONValueNewArray())) - goto error; + socks = virJSONValueNewArray(); if (virJSONValueObjectAppend(object, "socks", socks) < 0) { virJSONValueFree(socks); diff --git a/src/util/virjson.c b/src/util/virjson.c index 50993648eb..ca57df816f 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -561,10 +561,7 @@ virJSONValueNewNull(void) virJSONValuePtr virJSONValueNewArray(void) { - virJSONValuePtr val; - - if (VIR_ALLOC(val) < 0) - return NULL; + virJSONValuePtr val = g_new0(virJSONValue, 1); val->type = VIR_JSON_TYPE_ARRAY; @@ -1265,8 +1262,7 @@ virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap) virJSONValuePtr ret; ssize_t pos = -1; - if (!(ret = virJSONValueNewArray())) - return NULL; + ret = virJSONValueNewArray(); if (!bitmap) return ret; @@ -1522,8 +1518,6 @@ virJSONValueCopy(const virJSONValue *in) break; case VIR_JSON_TYPE_ARRAY: out = virJSONValueNewArray(); - if (!out) - return NULL; for (i = 0; i < in->data.array.nvalues; i++) { virJSONValuePtr val = NULL; if (!(val = virJSONValueCopy(in->data.array.values[i]))) @@ -1782,9 +1776,6 @@ virJSONParserHandleStartArray(void *ctx) VIR_DEBUG("parser=%p", parser); - if (!value) - return 0; - if (virJSONParserInsertValue(parser, value) < 0) { virJSONValueFree(value); return 0; diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 59d47daae8..a44377f89e 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -443,8 +443,7 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace) virJSONValueObjectAppendString(object, "directory", lockspace->dir) < 0) goto error; - if (!(resources = virJSONValueNewArray())) - goto error; + resources = virJSONValueNewArray(); if (virJSONValueObjectAppend(object, "resources", resources) < 0) { virJSONValueFree(resources); @@ -479,8 +478,7 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace) goto error; } - if (!(owners = virJSONValueNewArray())) - goto error; + owners = virJSONValueNewArray(); if (virJSONValueObjectAppend(child, "owners", owners) < 0) { virJSONValueFree(owners); diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index cd74f67678..ec589334ea 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -206,10 +206,11 @@ virMACMapHashDumper(void *payload, size_t i; int ret = -1; - if (!(obj = virJSONValueNewObject()) || - !(arr = virJSONValueNewArray())) + if (!(obj = virJSONValueNewObject())) goto cleanup; + arr = virJSONValueNewArray(); + for (i = 0; macs[i]; i++) { virJSONValuePtr m = virJSONValueNewString(macs[i]); @@ -244,8 +245,7 @@ virMacMapDumpStrLocked(virMacMapPtr mgr, virJSONValuePtr arr; int ret = -1; - if (!(arr = virJSONValueNewArray())) - goto cleanup; + arr = virJSONValueNewArray(); if (virHashForEach(mgr->macs, virMACMapHashDumper, arr) < 0) goto cleanup; diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 50dcb86810..cd61334b4a 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -737,8 +737,7 @@ testQemuCheckpointDeleteMerge(const void *opaque) return -1; } - if (!(actions = virJSONValueNewArray())) - return -1; + actions = virJSONValueNewArray(); if (qemuCheckpointDiscardDiskBitmaps(data->chain, nodedata, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0334f83628..2c696a2e8b 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2948,9 +2948,8 @@ testQemuMonitorJSONTransaction(const void *opaque) if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema))) return -1; - if (!(actions = virJSONValueNewArray()) || - !(mergebitmaps = virJSONValueNewArray())) - return -1; + actions = virJSONValueNewArray(); + mergebitmaps = virJSONValueNewArray(); if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, "node1", "bitmap1") < 0 || qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps, "node2", "bitmap2") < 0) -- 2.24.1

Fix all implementations of virHashKeyCopy to always return a valid pointer. Tweak the return value expectation comment so that it doesn't necessarily require to allocate memory. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_addr.c | 5 +---- src/util/virhash.c | 4 +--- src/util/virhash.h | 3 ++- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index f07b3d9725..e0be655772 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -997,10 +997,7 @@ virZPCIAddrKeyEqual(const void *namea, static void * virZPCIAddrKeyCopy(const void *name) { - unsigned int *copy; - - if (VIR_ALLOC(copy) < 0) - return NULL; + unsigned int *copy = g_new0(unsigned int, 1); *copy = *((unsigned int *)name); return (void *)copy; diff --git a/src/util/virhash.c b/src/util/virhash.c index d5c5e017a1..c57d9f8292 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -94,9 +94,7 @@ static bool virHashStrEqual(const void *namea, const void *nameb) static void *virHashStrCopy(const void *name) { - char *ret; - ret = g_strdup(name); - return ret; + return g_strdup(name); } diff --git a/src/util/virhash.h b/src/util/virhash.h index 08f99d8a3d..143ce52206 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -83,7 +83,8 @@ typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb); * Create a copy of the hash key, duplicating * memory allocation where applicable * - * Returns a newly allocated copy of @name + * Returns a copy of @name which will eventually be passed to the + * 'virHashKeyFree' callback at the end of it's lifetime. */ typedef void *(*virHashKeyCopy)(const void *name); /** -- 2.24.1

Use g_new0 and skip checking of the return value of keyCopy callback as both are bound to return a valid pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhash.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index c57d9f8292..36a2d312fc 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -344,7 +344,6 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, size_t key, len = 0; virHashEntryPtr entry; virHashEntryPtr last = NULL; - void *new_name; if ((table == NULL) || (name == NULL)) return -1; @@ -374,12 +373,8 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name, len++; } - if (VIR_ALLOC(entry) < 0 || !(new_name = table->keyCopy(name))) { - VIR_FREE(entry); - return -1; - } - - entry->name = new_name; + entry = g_new0(virHashEntry, 1); + entry->name = table->keyCopy(name); entry->payload = userdata; if (last) -- 2.24.1

Add a variable which will store the contents of the 'flags' variable as passed in by the individual block jobs. Since the flags may influence behaviour of the jobs it's important to preserve it to the finalization steps. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.h | 3 +++ src/qemu/qemu_domain.c | 7 +++++++ tests/qemustatusxml2xmldata/backup-pull-in.xml | 2 +- tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 8 ++++---- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 2f29e8209c..9e55382f15 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -129,6 +129,9 @@ struct _qemuBlockJobData { virStorageSourcePtr chain; /* Reference to the chain the job operates on. */ virStorageSourcePtr mirrorChain; /* reference to 'mirror' part of the job */ + unsigned int jobflags; /* per job flags */ + bool jobflagsmissing; /* job flags were not stored */ + union { qemuBlockJobPullData pull; qemuBlockJobCommitData commit; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 05a8d3de38..0c7a2641ea 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2582,6 +2582,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, virBufferEscapeString(&attrBuf, " newstate='%s'", newstate); if (job->brokentype != QEMU_BLOCKJOB_TYPE_NONE) virBufferEscapeString(&attrBuf, " brokentype='%s'", qemuBlockjobTypeToString(job->brokentype)); + if (!job->jobflagsmissing) + virBufferAsprintf(&attrBuf, " jobflags='0x%x'", job->jobflags); virBufferEscapeString(&childBuf, "<errmsg>%s</errmsg>", job->errmsg); if (job->disk) { @@ -3294,6 +3296,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, int newstate = -1; bool invalidData = false; xmlNodePtr tmp; + unsigned long jobflags = 0; ctxt->node = node; @@ -3333,6 +3336,9 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, STRNEQ(mirror, "yes")) invalidData = true; + if (virXPathULongHex("string(./@jobflags)", ctxt, &jobflags) != 0) + job->jobflagsmissing = true; + if (!disk && !invalidData) { if ((tmp = virXPathNode("./chains/disk", ctxt)) && !(job->chain = qemuDomainObjPrivateXMLParseBlockjobChain(tmp, ctxt, xmlopt))) @@ -3352,6 +3358,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, job->state = state; job->newstate = newstate; + job->jobflags = jobflags; job->errmsg = virXPathString("string(./errmsg)", ctxt); job->invalidData = invalidData; job->disk = disk; diff --git a/tests/qemustatusxml2xmldata/backup-pull-in.xml b/tests/qemustatusxml2xmldata/backup-pull-in.xml index 3c69c41840..1db978a3ac 100644 --- a/tests/qemustatusxml2xmldata/backup-pull-in.xml +++ b/tests/qemustatusxml2xmldata/backup-pull-in.xml @@ -235,7 +235,7 @@ <allowReboot value='yes'/> <nodename index='0'/> <blockjobs active='yes'> - <blockjob name='backup-vda-libvirt-3-format' type='backup' state='running'> + <blockjob name='backup-vda-libvirt-3-format' type='backup' state='running' jobflags='0x0'> <disk dst='vda'/> <bitmap name='bitmapname'/> <store type='file' format='qcow2'> diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index b5d62fd4ab..ca6d110179 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -261,19 +261,19 @@ </source> </src> </blockjob> - <blockjob name='copy-vdd-libvirt-4321-format' type='copy' state='ready' shallownew='yes'> + <blockjob name='copy-vdd-libvirt-4321-format' type='copy' state='ready' jobflags='0x0' shallownew='yes'> <disk dst='vdd'/> </blockjob> - <blockjob name='commit-vdc-libvirt-9-format' type='commit' state='running'> + <blockjob name='commit-vdc-libvirt-9-format' type='commit' state='running' jobflags='0x0'> <disk dst='vdc'/> <base node='libvirt-11-format'/> <top node='libvirt-9-format'/> <topparent node='libvirt-2-format'/> </blockjob> - <blockjob name='drive-virtio-disk0' type='copy' state='ready'> + <blockjob name='drive-virtio-disk0' type='copy' state='ready' jobflags='0x0'> <disk dst='vda' mirror='yes'/> </blockjob> - <blockjob name='create-libvirt-1338-format' type='create' state='running'> + <blockjob name='create-libvirt-1338-format' type='create' state='running' jobflags='0xabcd'> <chains> <disk type='file' format='qcow2'> <source file='/create/src1.qcow2' index='1339'> -- 2.24.1

The flags may control important aspects of the block job which may influence also the termination of the job. Store the 'flags' for all the block job types. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_blockjob.c | 13 ++++++++++--- src/qemu/qemu_blockjob.h | 9 ++++++--- src/qemu/qemu_driver.c | 7 ++++--- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 3dc9222a6f..6b59bbeb2c 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -252,7 +252,8 @@ qemuBlockJobDiskNew(virDomainObjPtr vm, qemuBlockJobDataPtr qemuBlockJobDiskNewPull(virDomainObjPtr vm, virDomainDiskDefPtr disk, - virStorageSourcePtr base) + virStorageSourcePtr base, + unsigned int jobflags) { qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(qemuBlockJobData) job = NULL; @@ -269,6 +270,7 @@ qemuBlockJobDiskNewPull(virDomainObjPtr vm, return NULL; job->data.pull.base = base; + job->jobflags = jobflags; if (qemuBlockJobRegister(job, vm, disk, true) < 0) return NULL; @@ -283,7 +285,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm, virStorageSourcePtr topparent, virStorageSourcePtr top, virStorageSourcePtr base, - bool delete_imgs) + bool delete_imgs, + unsigned int jobflags) { qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(qemuBlockJobData) job = NULL; @@ -307,6 +310,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm, job->data.commit.top = top; job->data.commit.base = base; job->data.commit.deleteCommittedImages = delete_imgs; + job->jobflags = jobflags; if (qemuBlockJobRegister(job, vm, disk, true) < 0) return NULL; @@ -350,7 +354,8 @@ qemuBlockJobDiskNewCopy(virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr mirror, bool shallow, - bool reuse) + bool reuse, + unsigned int jobflags) { qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(qemuBlockJobData) job = NULL; @@ -371,6 +376,8 @@ qemuBlockJobDiskNewCopy(virDomainObjPtr vm, if (shallow && !reuse) job->data.copy.shallownew = true; + job->jobflags = jobflags; + if (qemuBlockJobRegister(job, vm, disk, true) < 0) return NULL; diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 9e55382f15..72c7fa053e 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -176,7 +176,8 @@ qemuBlockJobDiskNew(virDomainObjPtr vm, qemuBlockJobDataPtr qemuBlockJobDiskNewPull(virDomainObjPtr vm, virDomainDiskDefPtr disk, - virStorageSourcePtr base); + virStorageSourcePtr base, + unsigned int jobflags); qemuBlockJobDataPtr qemuBlockJobDiskNewCommit(virDomainObjPtr vm, @@ -184,7 +185,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm, virStorageSourcePtr topparent, virStorageSourcePtr top, virStorageSourcePtr base, - bool delete_imgs); + bool delete_imgs, + unsigned int jobflags); qemuBlockJobDataPtr qemuBlockJobNewCreate(virDomainObjPtr vm, @@ -197,7 +199,8 @@ qemuBlockJobDiskNewCopy(virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr mirror, bool shallow, - bool reuse); + bool reuse, + unsigned int jobflags); qemuBlockJobDataPtr qemuBlockJobDiskNewBackup(virDomainObjPtr vm, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 021e5a2732..a2481232ad 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17706,7 +17706,7 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm, speed <<= 20; } - if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource))) + if (!(job = qemuBlockJobDiskNewPull(vm, disk, baseSource, flags))) goto endjob; if (blockdev) { @@ -18393,7 +18393,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, goto endjob; } - if (!(job = qemuBlockJobDiskNewCopy(vm, disk, mirror, mirror_shallow, mirror_reuse))) + if (!(job = qemuBlockJobDiskNewCopy(vm, disk, mirror, mirror_shallow, mirror_reuse, flags))) goto endjob; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; @@ -18814,7 +18814,8 @@ qemuDomainBlockCommit(virDomainPtr dom, if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource, baseSource, - flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE))) + flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE, + flags))) goto endjob; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; -- 2.24.1

Add a validator which checks that a bitmap spanning multiple backing chain members doesn't look broken. The current rules are that no intermediate birmaps are missing (unfortunately it's hard to know whether the topmost or bottommost bitmap is missing) and none of the components is inconsistent. We can obviously improve it over time. The validator is also tested against the existing bitmap data we have for the backup merging test as well as some of the existing broken bitmap synthetic test cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 41 +++++++++++++++++++++++++ src/qemu/qemu_block.h | 5 ++++ tests/qemublocktest.c | 70 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 03f029368e..b19290e677 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2687,3 +2687,44 @@ qemuBlockGetNamedNodeData(virDomainObjPtr vm, return g_steal_pointer(&blockNamedNodeData); } + + +/** + * qemuBlockBitmapChainIsValid: + * + * Validates that the backing chain of @src contains proper consistent bitmap + * data for a chain of bitmaps named @bitmapname. + * + * A valid chain: + * 1) bitmaps of same name are in a consecutive subset of images without gap + * 2) don't have any inconsistent bitmaps + */ +bool +qemuBlockBitmapChainIsValid(virStorageSourcePtr src, + const char *bitmapname, + virHashTablePtr blockNamedNodeData) +{ + qemuBlockNamedNodeDataBitmapPtr bitmap; + virStorageSourcePtr n; + bool chain_started = false; + bool chain_ended = false; + + for (n = src; n; n = n->backingStore) { + if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData, n, bitmapname))) { + if (chain_started) + chain_ended = true; + + continue; + } + + if (chain_ended) + return false; + + chain_started = true; + + if (bitmap->inconsistent) + return false; + } + + return chain_started; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 68646cbf2e..cf51b9bf4e 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -212,3 +212,8 @@ qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData, virHashTablePtr qemuBlockGetNamedNodeData(virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); + +bool +qemuBlockBitmapChainIsValid(virStorageSourcePtr src, + const char *bitmapname, + virHashTablePtr blockNamedNodeData); diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index cd61334b4a..133372908a 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -768,6 +768,41 @@ testQemuCheckpointDeleteMerge(const void *opaque) } +struct testQemuBlockBitmapValidateData { + const char *name; + const char *bitmapname; + virStorageSourcePtr chain; + bool expect; +}; + +static int +testQemuBlockBitmapValidate(const void *opaque) +{ + const struct testQemuBlockBitmapValidateData *data = opaque; + g_autoptr(virJSONValue) nodedatajson = NULL; + g_autoptr(virHashTable) nodedata = NULL; + bool actual; + + if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, data->name, + ".json", NULL))) + return -1; + + if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) { + VIR_TEST_VERBOSE("failed to load nodedata JSON\n"); + return -1; + } + + actual = qemuBlockBitmapChainIsValid(data->chain, data->bitmapname, nodedata); + + if (actual != data->expect) { + VIR_TEST_VERBOSE("expected rv:'%d' actual rv:'%d'\n", data->expect, actual); + return -1; + } + + return 0; +} + + static int mymain(void) { @@ -778,6 +813,7 @@ mymain(void) struct testQemuImageCreateData imagecreatedata; struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata; struct testQemuCheckpointDeleteMergeData checkpointdeletedata; + struct testQemuBlockBitmapValidateData blockbitmapvalidatedata; char *capslatest_x86_64 = NULL; virQEMUCapsPtr caps_x86_64 = NULL; g_autoptr(virStorageSource) bitmapSourceChain = NULL; @@ -1045,7 +1081,41 @@ mymain(void) TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate3", "d", "c", "snapshots-synthetic-checkpoint"); TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-current", "current", "d", "snapshots-synthetic-checkpoint"); +#define TEST_BITMAP_VALIDATE(testname, bitmap, rc) \ + do { \ + blockbitmapvalidatedata.name = testname; \ + blockbitmapvalidatedata.chain = bitmapSourceChain; \ + blockbitmapvalidatedata.bitmapname = bitmap; \ + blockbitmapvalidatedata.expect = rc; \ + if (virTestRun("bitmap validate " testname " " bitmap, \ + testQemuBlockBitmapValidate, \ + &blockbitmapvalidatedata) < 0) \ + ret = -1; \ + } while (0) + TEST_BITMAP_VALIDATE("basic", "a", true); + TEST_BITMAP_VALIDATE("basic", "b", true); + TEST_BITMAP_VALIDATE("basic", "c", true); + TEST_BITMAP_VALIDATE("basic", "d", true); + TEST_BITMAP_VALIDATE("basic", "current", true); + + TEST_BITMAP_VALIDATE("snapshots", "a", true); + TEST_BITMAP_VALIDATE("snapshots", "b", true); + TEST_BITMAP_VALIDATE("snapshots", "c", true); + TEST_BITMAP_VALIDATE("snapshots", "d", true); + TEST_BITMAP_VALIDATE("snapshots", "current", true); + + TEST_BITMAP_VALIDATE("synthetic", "a", false); + TEST_BITMAP_VALIDATE("synthetic", "b", true); + TEST_BITMAP_VALIDATE("synthetic", "c", true); + TEST_BITMAP_VALIDATE("synthetic", "d", true); + TEST_BITMAP_VALIDATE("synthetic", "current", true); + + TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "a", true); + TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "b", true); + TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "c", true); + TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "d", true); + TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "current", true); cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); -- 2.24.1

Add a case where a bitmap spanning multiple images is missing one of the intermediate components. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 8 + .../bitmap/snapshots-synthetic-broken.json | 819 ++++++++++++++++++ .../bitmap/snapshots-synthetic-broken.out | 12 + 3 files changed, 839 insertions(+) create mode 100644 tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.out diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 133372908a..ada3608e53 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1030,6 +1030,7 @@ mymain(void) TEST_BITMAP_DETECT("synthetic"); TEST_BITMAP_DETECT("snapshots"); TEST_BITMAP_DETECT("snapshots-synthetic-checkpoint"); + TEST_BITMAP_DETECT("snapshots-synthetic-broken"); #define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \ do { \ @@ -1116,6 +1117,13 @@ mymain(void) TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "c", true); TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "d", true); TEST_BITMAP_VALIDATE("snapshots-synthetic-checkpoint", "current", true); + + TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "a", false); + TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "b", true); + TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "c", true); + TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "d", false); + TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "current", true); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json b/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json new file mode 100644 index 0000000000..bf4963494f --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.json @@ -0,0 +1,819 @@ +[ + { + "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, + "inconsistent": true, + "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": true, + "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.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": [ + ], + "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-synthetic-broken.out b/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.out new file mode 100644 index 0000000000..022630bd76 --- /dev/null +++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-broken.out @@ -0,0 +1,12 @@ +libvirt-1-format: + d: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 + current: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 +libvirt-2-format: + c: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 + d: record:1 busy:0 persist:1 inconsist:1 gran:65536 dirty:0 +libvirt-3-format: + a: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 + b: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 +libvirt-4-format: +libvirt-5-format: + a: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 -- 2.24.1

Add a function calculating which bitmaps to copy to the mirror during a block-copy operation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 138 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 7 +++ 2 files changed, 145 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b19290e677..63116ef5f2 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2728,3 +2728,141 @@ qemuBlockBitmapChainIsValid(virStorageSourcePtr src, return chain_started; } + + +struct qemuBlockBitmapsHandleBlockcopyConcatData { + virHashTablePtr bitmaps_merge; + virJSONValuePtr actions; + const char *mirrornodeformat; + bool has_bitmaps; +}; + + +static int +qemuBlockBitmapsHandleBlockcopyConcatActions(void *payload, + const void *name, + void *opaque) +{ + struct qemuBlockBitmapsHandleBlockcopyConcatData *data = opaque; + virJSONValuePtr createactions = payload; + const char *bitmapname = name; + g_autoptr(virJSONValue) mergebitmaps = virHashSteal(data->bitmaps_merge, bitmapname); + + data->has_bitmaps = true; + + virJSONValueArrayConcat(data->actions, createactions); + + if (qemuMonitorTransactionBitmapMerge(data->actions, + data->mirrornodeformat, + bitmapname, + &mergebitmaps) < 0) + return -1; + + return 0; +} + + +/** + * qemuBlockBitmapsHandleBlockcopy: + * @src: disk source + * @mirror: mirror source + * @blockNamedNodeData: hash table containing data about bitmaps + * @shallow: whether shallow copy is requested + * @actions: filled with arguments for a 'transaction' command + * + * Calculates which bitmaps to copy and merge during a virDomainBlockCopy job. + * This is designed to be called when the job is already synchronised as it + * may result in active bitmaps being created. + * + * Returns 0 on success and -1 on error. If @actions is NULL when 0 is returned + * there are no actions to perform for the given job. + */ +int +qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src, + virStorageSourcePtr mirror, + virHashTablePtr blockNamedNodeData, + bool shallow, + virJSONValuePtr *actions) +{ + g_autoptr(virHashTable) bitmaps = virHashNew(virJSONValueHashFree); + g_autoptr(virHashTable) bitmaps_merge = virHashNew(virJSONValueHashFree); + g_autoptr(virHashTable) bitmaps_skip = virHashNew(NULL); + g_autoptr(virJSONValue) tmpactions = virJSONValueNewArray(); + qemuBlockNamedNodeDataPtr entry; + virStorageSourcePtr n; + size_t i; + struct qemuBlockBitmapsHandleBlockcopyConcatData data = { .bitmaps_merge = bitmaps_merge, + .actions = tmpactions, + .mirrornodeformat = mirror->nodeformat, + .has_bitmaps = false, }; + + for (n = src; n; n = n->backingStore) { + if (!(entry = virHashLookup(blockNamedNodeData, n->nodeformat))) + continue; + + for (i = 0; i < entry->nbitmaps; i++) { + qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i]; + virJSONValuePtr bitmap_merge; + + if (virHashHasEntry(bitmaps_skip, bitmap->name)) + continue; + + if (!(bitmap_merge = virHashLookup(bitmaps_merge, bitmap->name))) { + g_autoptr(virJSONValue) tmp = NULL; + bool disabled = !bitmap->recording; + + /* disable any non top-layer bitmaps */ + if (n != src) + disabled = true; + + if (!bitmap->persistent || + !(qemuBlockBitmapChainIsValid(n, bitmap->name, + blockNamedNodeData))) { + ignore_value(virHashAddEntry(bitmaps_skip, bitmap->name, NULL)); + continue; + } + + /* prepare the data for adding the bitmap to the mirror */ + tmp = virJSONValueNewArray(); + + if (qemuMonitorTransactionBitmapAdd(tmp, + mirror->nodeformat, + bitmap->name, + true, + disabled, + bitmap->granularity) < 0) + return -1; + + if (virHashAddEntry(bitmaps, bitmap->name, tmp) < 0) + return -1; + + tmp = NULL; + + /* prepare array for merging all the bitmaps from the original chain */ + tmp = virJSONValueNewArray(); + + if (virHashAddEntry(bitmaps_merge, bitmap->name, tmp) < 0) + return -1; + + bitmap_merge = g_steal_pointer(&tmp); + } + + if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(bitmap_merge, + n->nodeformat, + bitmap->name) < 0) + return -1; + } + + if (shallow) + break; + } + + if (virHashForEach(bitmaps, qemuBlockBitmapsHandleBlockcopyConcatActions, + &data) < 0) + return -1; + + if (data.has_bitmaps) + *actions = g_steal_pointer(&tmpactions); + + return 0; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index cf51b9bf4e..a816190bb7 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -217,3 +217,10 @@ bool qemuBlockBitmapChainIsValid(virStorageSourcePtr src, const char *bitmapname, virHashTablePtr blockNamedNodeData); + +int +qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src, + virStorageSourcePtr mirror, + virHashTablePtr blockNamedNodeData, + bool shallow, + virJSONValuePtr *actions); -- 2.24.1

Use some of the existing bitmap data to add tests for qemuBlockBitmapsHandleBlockcopy. As the output depends on the ordering in the hash table we must also install the "virdeterministichash" mock preload library. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 71 +++++++++- .../bitmapblockcopy/basic-deep-out.json | 117 +++++++++++++++ .../bitmapblockcopy/basic-shallow-out.json | 117 +++++++++++++++ .../bitmapblockcopy/snapshots-deep-out.json | 133 ++++++++++++++++++ .../snapshots-shallow-out.json | 48 +++++++ 5 files changed, 485 insertions(+), 1 deletion(-) create mode 100644 tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json create mode 100644 tests/qemublocktestdata/bitmapblockcopy/basic-shallow-out.json create mode 100644 tests/qemublocktestdata/bitmapblockcopy/snapshots-deep-out.json create mode 100644 tests/qemublocktestdata/bitmapblockcopy/snapshots-shallow-out.json diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index ada3608e53..06d89b9230 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -803,6 +803,56 @@ testQemuBlockBitmapValidate(const void *opaque) } +static const char *blockcopyPrefix = "qemublocktestdata/bitmapblockcopy/"; + +struct testQemuBlockBitmapBlockcopyData { + const char *name; + bool shallow; + virStorageSourcePtr chain; + const char *nodedatafile; +}; + + +static int +testQemuBlockBitmapBlockcopy(const void *opaque) +{ + const struct testQemuBlockBitmapBlockcopyData *data = opaque; + g_autofree char *actual = NULL; + g_autofree char *expectpath = NULL; + g_autoptr(virJSONValue) actions = NULL; + g_autoptr(virJSONValue) nodedatajson = NULL; + g_autoptr(virHashTable) nodedata = NULL; + g_autoptr(virStorageSource) fakemirror = virStorageSourceNew(); + + if (!fakemirror) + return -1; + + fakemirror->nodeformat = g_strdup("mirror-format-node"); + + expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir, + blockcopyPrefix, 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; + } + + if (qemuBlockBitmapsHandleBlockcopy(data->chain, fakemirror, nodedata, + data->shallow, &actions) < 0) + return -1; + + if (actions && + !(actual = virJSONValueToString(actions, true))) + return -1; + + return virTestCompareToFile(actual, expectpath); +} + + static int mymain(void) { @@ -814,6 +864,7 @@ mymain(void) struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata; struct testQemuCheckpointDeleteMergeData checkpointdeletedata; struct testQemuBlockBitmapValidateData blockbitmapvalidatedata; + struct testQemuBlockBitmapBlockcopyData blockbitmapblockcopydata; char *capslatest_x86_64 = NULL; virQEMUCapsPtr caps_x86_64 = NULL; g_autoptr(virStorageSource) bitmapSourceChain = NULL; @@ -1124,6 +1175,24 @@ mymain(void) TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "d", false); TEST_BITMAP_VALIDATE("snapshots-synthetic-broken", "current", true); +#define TEST_BITMAP_BLOCKCOPY(testname, shllw, ndf) \ + do { \ + blockbitmapblockcopydata.name = testname; \ + blockbitmapblockcopydata.shallow = shllw; \ + blockbitmapblockcopydata.nodedatafile = ndf; \ + blockbitmapblockcopydata.chain = bitmapSourceChain;\ + if (virTestRun("bitmap block copy " testname, \ + testQemuBlockBitmapBlockcopy, \ + &blockbitmapblockcopydata) < 0) \ + ret = -1; \ + } while (0) + + TEST_BITMAP_BLOCKCOPY("basic-shallow", true, "basic"); + TEST_BITMAP_BLOCKCOPY("basic-deep", false, "basic"); + + TEST_BITMAP_BLOCKCOPY("snapshots-shallow", true, "snapshots"); + TEST_BITMAP_BLOCKCOPY("snapshots-deep", false, "snapshots"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); @@ -1133,4 +1202,4 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN(mymain) +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdeterministichash")) diff --git a/tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json b/tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json new file mode 100644 index 0000000000..4ed2b97e95 --- /dev/null +++ b/tests/qemublocktestdata/bitmapblockcopy/basic-deep-out.json @@ -0,0 +1,117 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "a", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "a", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "a" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "b", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "b" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "current", + "persistent": true, + "disabled": false, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "current", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "c", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "d", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + } + ] + } + } +] diff --git a/tests/qemublocktestdata/bitmapblockcopy/basic-shallow-out.json b/tests/qemublocktestdata/bitmapblockcopy/basic-shallow-out.json new file mode 100644 index 0000000000..4ed2b97e95 --- /dev/null +++ b/tests/qemublocktestdata/bitmapblockcopy/basic-shallow-out.json @@ -0,0 +1,117 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "a", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "a", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "a" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "b", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "b" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "current", + "persistent": true, + "disabled": false, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "current", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "c", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "d", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + } + ] + } + } +] diff --git a/tests/qemublocktestdata/bitmapblockcopy/snapshots-deep-out.json b/tests/qemublocktestdata/bitmapblockcopy/snapshots-deep-out.json new file mode 100644 index 0000000000..5456553d78 --- /dev/null +++ b/tests/qemublocktestdata/bitmapblockcopy/snapshots-deep-out.json @@ -0,0 +1,133 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "a", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "a", + "bitmaps": [ + { + "node": "libvirt-3-format", + "name": "a" + }, + { + "node": "libvirt-4-format", + "name": "a" + }, + { + "node": "libvirt-5-format", + "name": "a" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "b", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "b", + "bitmaps": [ + { + "node": "libvirt-3-format", + "name": "b" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "current", + "persistent": true, + "disabled": false, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "current", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "c", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "c", + "bitmaps": [ + { + "node": "libvirt-2-format", + "name": "c" + }, + { + "node": "libvirt-3-format", + "name": "c" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "d", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + }, + { + "node": "libvirt-2-format", + "name": "d" + } + ] + } + } +] diff --git a/tests/qemublocktestdata/bitmapblockcopy/snapshots-shallow-out.json b/tests/qemublocktestdata/bitmapblockcopy/snapshots-shallow-out.json new file mode 100644 index 0000000000..ddd47f7ee1 --- /dev/null +++ b/tests/qemublocktestdata/bitmapblockcopy/snapshots-shallow-out.json @@ -0,0 +1,48 @@ +[ + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "current", + "persistent": true, + "disabled": false, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "current", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "current" + } + ] + } + }, + { + "type": "block-dirty-bitmap-add", + "data": { + "node": "mirror-format-node", + "name": "d", + "persistent": true, + "disabled": true, + "granularity": 65536 + } + }, + { + "type": "block-dirty-bitmap-merge", + "data": { + "node": "mirror-format-node", + "target": "d", + "bitmaps": [ + { + "node": "libvirt-1-format", + "name": "d" + } + ] + } + } +] -- 2.24.1

Use qemuBlockBitmapsHandleBlockcopy to calculate bitmaps to copy over for a block-copy job. We copy them when pivoting to the new image as at that point we are certain that we don't dirty any bitmap unnecessarily. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a2481232ad..7df3f53183 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17572,6 +17572,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); + g_autoptr(virJSONValue) actions = NULL; switch ((qemuBlockJobType) job->type) { case QEMU_BLOCKJOB_TYPE_NONE: @@ -17592,6 +17593,20 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, return -1; case QEMU_BLOCKJOB_TYPE_COPY: + if (blockdev && !job->jobflagsmissing) { + g_autoptr(virHashTable) blockNamedNodeData = NULL; + bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW; + + if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE))) + return -1; + + if (qemuBlockBitmapsHandleBlockcopy(disk->src, disk->mirror, + blockNamedNodeData, + shallow, &actions) < 0) + return -1; + } + break; + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: break; } @@ -17604,10 +17619,17 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, } qemuDomainObjEnterMonitor(driver, vm); - if (blockdev) - ret = qemuMonitorJobComplete(priv->mon, job->name); - else + if (blockdev) { + int rc = 0; + + if (actions) + rc = qemuMonitorTransaction(priv->mon, &actions); + + if (rc == 0) + ret = qemuMonitorJobComplete(priv->mon, job->name); + } else { ret = qemuMonitorDrivePivot(priv->mon, job->name); + } if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; -- 2.24.1