[libvirt] [RFC PATCH 00/16] qemu: checkpoint: Add support for deleting checkpoints accross snapshots
12:21 p.m.
Posted as RFC because qemu probably doesn't reopen backing images for
bitmap operations resulting in:
$ virsh checkpoint-delete vm --checkpointname a
error: Failed to delete checkpoint a
error: internal error: unable to execute QEMU command 'transaction': Bitmap
'a' is readonly and cannot be modified
Unfortunately this can't be done manually because 'blockdev-reopen' is
still experimental.
Peter Krempa (16):
qemu: checkpoint: Store whether deleted checkpoint is current in a
variable
qemu: checkpoint: split out checkpoint deletion bitmaps
qemu: checkpoint: rename disk->chkdisk in qemuCheckpointDiscardBitmaps
qemu: checkpoint: rename disk->chkdisk in qemuCheckpointAddActions
qemu: checkpoint: Use disk definition directly when creating
checkpoint
qemu: checkpoint: tolerate missing disks on checkpoint deletion
qemu: domain: Remove unused qemuDomainDiskNodeFormatLookup
qemu: checkpoint: Introduce helper to find checkpoint disk definition
in parents
qemu: checkpoint: Extract calculation of bitmap merging for checkpoint
deletion
tests: qemublock: Add test for checkpoint deletion bitmap merge
tests: qemublock: Add few more test cases for checkpoint deletion
tests: qemublock: Add synthetic snapshot+checkpoint test data
qemu: checkpoint: Introduce support for deleting checkpoints accross
snapshots
tests: qemublock: Add checkpoint deletion test for deep backing chain
tests: qemublock: Add checkpoint deletion tests for some special cases
qemu: checkpoint: Track and relabel images for bitmap merging
src/qemu/qemu_checkpoint.c | 329 +++++--
src/qemu/qemu_checkpoint.h | 9 +
src/qemu/qemu_domain.c | 14 -
src/qemu/qemu_domain.h | 3 -
tests/qemublocktest.c | 102 +++
.../snapshots-synthetic-checkpoint.json | 827 ++++++++++++++++++
.../bitmap/snapshots-synthetic-checkpoint.out | 13 +
.../checkpointdelete/basic-current-out.json | 29 +
.../basic-intermediate1-out.json | 22 +
.../basic-intermediate2-out.json | 22 +
.../basic-intermediate3-out.json | 22 +
.../checkpointdelete/basic-noparent-out.json | 9 +
.../snapshots-current-out.json | 29 +
.../snapshots-intermediate1-out.json | 24 +
.../snapshots-intermediate2-out.json | 62 ++
.../snapshots-intermediate3-out.json | 61 ++
.../snapshots-noparent-out.json | 27 +
...hots-synthetic-checkpoint-current-out.json | 29 +
...ynthetic-checkpoint-intermediate1-out.json | 31 +
...ynthetic-checkpoint-intermediate2-out.json | 34 +
...ynthetic-checkpoint-intermediate3-out.json | 61 ++
...ots-synthetic-checkpoint-noparent-out.json | 27 +
22 files changed, 1680 insertions(+), 106 deletions(-)
create mode 100644 tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
create mode 100644 tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out
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
create mode 100644 tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json
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
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
--
2.24.1
12:21 p.m.
New subject: [libvirt] [RFC PATCH 01/16] qemu: checkpoint: Store whether deleted checkpoint is current in a variable
Avoid two computations by using a boolean.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 2fa5c1ae00..d13d4c2a37 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -117,6 +117,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
size_t i, j;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *chkFile = NULL;
+ bool chkcurrent = chk == virDomainCheckpointGetCurrent(vm->checkpoints);
if (!metadata_only && !virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -172,7 +173,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node,
disk->bitmap) < 0)
return -1;
- if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) {
+ if (chkcurrent) {
if (qemuMonitorTransactionBitmapEnable(actions, node,
disk2->bitmap) < 0)
return -1;
}
@@ -192,7 +193,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
return -1;
}
- if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) {
+ if (chkcurrent) {
virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
if (update_parent && parent) {
virDomainCheckpointSetCurrent(vm->checkpoints, parent);
--
2.24.1
10:50 a.m.
New subject: [RFC PATCH 01/16] qemu: checkpoint: Store whether deleted checkpoint is current in a variable
On 1/9/20 12:21 PM, Peter Krempa wrote:
Avoid two computations by using a boolean.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 02/16] qemu: checkpoint: split out checkpoint deletion bitmaps
qemuCheckpointDiscard is a massive function that can be separated into
smaller bits. Extract the part that actually modifies the disk from the
metadata handling.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 137 ++++++++++++++++++++-----------------
1 file changed, 76 insertions(+), 61 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index d13d4c2a37..9ff3129570 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -104,6 +104,81 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
}
+static int
+qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
+ virDomainCheckpointDefPtr chkdef,
+ bool chkcurrent,
+ virDomainMomentObjPtr parent)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverPtr driver = priv->driver;
+ virDomainMomentObjPtr moment;
+ virDomainCheckpointDefPtr parentdef = NULL;
+ bool search_parents;
+ int rc;
+ g_autoptr(virJSONValue) actions = NULL;
+ size_t i;
+ size_t j;
+
+ if (!(actions = virJSONValueNewArray()))
+ return -1;
+
+ for (i = 0; i < chkdef->ndisks; i++) {
+ virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
+ const char *node;
+
+ if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ continue;
+
+ node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
+ /* If any ancestor checkpoint has a bitmap for the same
+ * disk, then this bitmap must be merged to the
+ * ancestor. */
+ search_parents = true;
+ for (moment = parent;
+ search_parents && moment;
+ moment = virDomainCheckpointFindByName(vm->checkpoints,
+ parentdef->parent.parent_name)) {
+ parentdef = virDomainCheckpointObjGetDef(moment);
+ for (j = 0; j < parentdef->ndisks; j++) {
+ virDomainCheckpointDiskDef *disk2;
+ g_autoptr(virJSONValue) arr = NULL;
+
+ disk2 = &parentdef->disks[j];
+ if (STRNEQ(disk->name, disk2->name) ||
+ disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ continue;
+ search_parents = false;
+
+ if (!(arr = virJSONValueNewArray()))
+ return -1;
+
+ if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node,
disk->bitmap) < 0)
+ return -1;
+
+ if (chkcurrent) {
+ if (qemuMonitorTransactionBitmapEnable(actions, node,
disk2->bitmap) < 0)
+ return -1;
+ }
+
+ if (qemuMonitorTransactionBitmapMerge(actions, node, disk2->bitmap,
&arr) < 0)
+ return -1;
+ }
+ }
+
+ if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) < 0)
+ return -1;
+ }
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ rc = qemuMonitorTransaction(priv->mon, &actions);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+ return -1;
+
+ return 0;
+}
+
+
static int
qemuCheckpointDiscard(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -112,9 +187,6 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
bool metadata_only)
{
virDomainMomentObjPtr parent = NULL;
- virDomainMomentObjPtr moment;
- virDomainCheckpointDefPtr parentdef = NULL;
- size_t i, j;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *chkFile = NULL;
bool chkcurrent = chk == virDomainCheckpointGetCurrent(vm->checkpoints);
@@ -129,67 +201,10 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
chk->def->name);
if (!metadata_only) {
- qemuDomainObjPrivatePtr priv = vm->privateData;
- bool search_parents;
virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
- int rc;
- g_autoptr(virJSONValue) actions = NULL;
-
- if (!(actions = virJSONValueNewArray()))
- return -1;
-
parent = virDomainCheckpointFindByName(vm->checkpoints,
chk->def->parent_name);
- for (i = 0; i < chkdef->ndisks; i++) {
- virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
- const char *node;
-
- if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
- continue;
-
- node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
- /* If any ancestor checkpoint has a bitmap for the same
- * disk, then this bitmap must be merged to the
- * ancestor. */
- search_parents = true;
- for (moment = parent;
- search_parents && moment;
- moment = virDomainCheckpointFindByName(vm->checkpoints,
-
parentdef->parent.parent_name)) {
- parentdef = virDomainCheckpointObjGetDef(moment);
- for (j = 0; j < parentdef->ndisks; j++) {
- virDomainCheckpointDiskDef *disk2;
- g_autoptr(virJSONValue) arr = NULL;
-
- disk2 = &parentdef->disks[j];
- if (STRNEQ(disk->name, disk2->name) ||
- disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
- continue;
- search_parents = false;
-
- if (!(arr = virJSONValueNewArray()))
- return -1;
-
- if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node,
disk->bitmap) < 0)
- return -1;
-
- if (chkcurrent) {
- if (qemuMonitorTransactionBitmapEnable(actions, node,
disk2->bitmap) < 0)
- return -1;
- }
-
- if (qemuMonitorTransactionBitmapMerge(actions, node,
disk2->bitmap, &arr) < 0)
- return -1;
- }
- }
-
- if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) <
0)
- return -1;
- }
-
- qemuDomainObjEnterMonitor(driver, vm);
- rc = qemuMonitorTransaction(priv->mon, &actions);
- if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+ if (qemuCheckpointDiscardBitmaps(vm, chkdef, chkcurrent, parent) < 0)
return -1;
}
--
2.24.1
11:13 a.m.
New subject: [RFC PATCH 02/16] qemu: checkpoint: split out checkpoint deletion bitmaps
On 1/9/20 12:21 PM, Peter Krempa wrote:
qemuCheckpointDiscard is a massive function that can be separated
into
smaller bits. Extract the part that actually modifies the disk from the
metadata handling.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 137 ++++++++++++++++++++-----------------
1 file changed, 76 insertions(+), 61 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index d13d4c2a37..9ff3129570 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -104,6 +104,81 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
}
+static int
+qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
+ virDomainCheckpointDefPtr chkdef,
+ bool chkcurrent,
+ virDomainMomentObjPtr parent)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverPtr driver = priv->driver;
+ virDomainMomentObjPtr moment;
+ virDomainCheckpointDefPtr parentdef = NULL;
+ bool search_parents;
+ int rc;
+ g_autoptr(virJSONValue) actions = NULL;
+ size_t i;
+ size_t j;
+
+ if (!(actions = virJSONValueNewArray()))
+ return -1;
Can virJSONValueNewArray() still return failure now that we have shifted
to using g_new()?
But changing it (if needed) should be independent of the code motion
shown here.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
3:08 a.m.
New subject: [RFC PATCH 02/16] qemu: checkpoint: split out checkpoint deletion bitmaps
On Fri, Jan 24, 2020 at 11:13:01 -0600, Eric Blake wrote:
On 1/9/20 12:21 PM, Peter Krempa wrote:
> qemuCheckpointDiscard is a massive function that can be separated into
> smaller bits. Extract the part that actually modifies the disk from the
> metadata handling.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_checkpoint.c | 137 ++++++++++++++++++++-----------------
> 1 file changed, 76 insertions(+), 61 deletions(-)
>
> diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
> index d13d4c2a37..9ff3129570 100644
> --- a/src/qemu/qemu_checkpoint.c
> +++ b/src/qemu/qemu_checkpoint.c
> @@ -104,6 +104,81 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
> }
>
>
> +static int
> +qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
> + virDomainCheckpointDefPtr chkdef,
> + bool chkcurrent,
> + virDomainMomentObjPtr parent)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + virQEMUDriverPtr driver = priv->driver;
> + virDomainMomentObjPtr moment;
> + virDomainCheckpointDefPtr parentdef = NULL;
> + bool search_parents;
> + int rc;
> + g_autoptr(virJSONValue) actions = NULL;
> + size_t i;
> + size_t j;
> +
> + if (!(actions = virJSONValueNewArray()))
> + return -1;
Can virJSONValueNewArray() still return failure now that we have shifted to
using g_new()?
It can't fail any more, but it internally uses VIR_ALLOC which is
declared with ATTRIBUTE_RETURN_CHECK thus we have to check the return
value. Also even if we did not, e.g. coverity heuristically assumes that
if a big number of callers check the return value and moans if some
don't.
That said. I can add a ignore_value().
But changing it (if needed) should be independent of the code motion shown
here.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 03/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointDiscardBitmaps
Upcomming patches will also use the domain diks definition. Rename disk
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 9ff3129570..d347b8fc6c 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -124,13 +124,13 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
return -1;
for (i = 0; i < chkdef->ndisks; i++) {
- virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
+ virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];
const char *node;
- if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
- node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
+ node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
/* If any ancestor checkpoint has a bitmap for the same
* disk, then this bitmap must be merged to the
* ancestor. */
@@ -145,7 +145,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
g_autoptr(virJSONValue) arr = NULL;
disk2 = &parentdef->disks[j];
- if (STRNEQ(disk->name, disk2->name) ||
+ if (STRNEQ(chkdisk->name, disk2->name) ||
disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
search_parents = false;
@@ -153,7 +153,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
if (!(arr = virJSONValueNewArray()))
return -1;
- if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node,
disk->bitmap) < 0)
+ if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node,
chkdisk->bitmap) < 0)
return -1;
if (chkcurrent) {
@@ -166,7 +166,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
}
}
- if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) < 0)
+ if (qemuMonitorTransactionBitmapRemove(actions, node, chkdisk->bitmap) <
0)
return -1;
}
--
2.24.1
11:15 a.m.
New subject: [RFC PATCH 03/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointDiscardBitmaps
On 1/9/20 12:21 PM, Peter Krempa wrote:
Upcomming patches will also use the domain diks definition. Rename
disk
Upcoming, disck
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Mechanical, other than the commit message typos.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:30 p.m.
New subject: [RFC PATCH 03/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointDiscardBitmaps
On 1/24/20 11:15 AM, Eric Blake wrote:
On 1/9/20 12:21 PM, Peter Krempa wrote:
> Upcomming patches will also use the domain diks definition. Rename disk
Upcoming, disck
Urgh - I typo'd my typo correction. 'disck' is not a word, and 'disc'
is unusual these days; so I meant 'disk'.
> to chkdisk for clarity.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> Â src/qemu/qemu_checkpoint.c | 12 ++++++------
> Â 1 file changed, 6 insertions(+), 6 deletions(-)
Mechanical, other than the commit message typos.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 04/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointAddActions
Upcomming patches will also use the domain diks definition. Rename disk
to chkdisk for clarity.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index d347b8fc6c..0aa854324b 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -319,13 +319,13 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
bool search_parents;
for (i = 0; i < def->ndisks; i++) {
- virDomainCheckpointDiskDef *disk = &def->disks[i];
+ virDomainCheckpointDiskDef *chkdisk = &def->disks[i];
const char *node;
- if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
- node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
- if (qemuMonitorTransactionBitmapAdd(actions, node, disk->bitmap, true, false,
0) < 0)
+ node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
+ if (qemuMonitorTransactionBitmapAdd(actions, node, chkdisk->bitmap, true,
false, 0) < 0)
return -1;
/* We only want one active bitmap for a disk along the
@@ -345,7 +345,7 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
virDomainCheckpointDiskDef *disk2;
disk2 = &olddef->disks[j];
- if (STRNEQ(disk->name, disk2->name) ||
+ if (STRNEQ(chkdisk->name, disk2->name) ||
disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
if (qemuMonitorTransactionBitmapDisable(actions, node, disk2->bitmap)
< 0)
--
2.24.1
11:16 a.m.
New subject: [RFC PATCH 04/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointAddActions
On 1/9/20 12:21 PM, Peter Krempa wrote:
Upcomming patches will also use the domain diks definition. Rename
disk
to chkdisk for clarity.
Same typos as in 3/16. You could squash the two patches if desired,
since it is so mechanical.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 05/16] qemu: checkpoint: Use disk definition directly when creating checkpoint
Lookup the whole disk definition rather than just the node name.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 0aa854324b..03a8321135 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -320,12 +320,16 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
for (i = 0; i < def->ndisks; i++) {
virDomainCheckpointDiskDef *chkdisk = &def->disks[i];
- const char *node;
+ virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def,
chkdisk->name);
- if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ /* checkpoint definition validator mandates that the corresponding
+ * domdisk should exist */
+ if (!domdisk ||
+ chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
- node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
- if (qemuMonitorTransactionBitmapAdd(actions, node, chkdisk->bitmap, true,
false, 0) < 0)
+
+ if (qemuMonitorTransactionBitmapAdd(actions, domdisk->src->nodeformat,
+ chkdisk->bitmap, true, false, 0) < 0)
return -1;
/* We only want one active bitmap for a disk along the
@@ -348,7 +352,9 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
if (STRNEQ(chkdisk->name, disk2->name) ||
disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
- if (qemuMonitorTransactionBitmapDisable(actions, node, disk2->bitmap)
< 0)
+ if (qemuMonitorTransactionBitmapDisable(actions,
+ domdisk->src->nodeformat,
+ disk2->bitmap) < 0)
return -1;
search_parents = false;
break;
--
2.24.1
12:32 p.m.
New subject: [RFC PATCH 05/16] qemu: checkpoint: Use disk definition directly when creating checkpoint
On 1/9/20 12:21 PM, Peter Krempa wrote:
Lookup the whole disk definition rather than just the node name.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 06/16] qemu: checkpoint: tolerate missing disks on checkpoint deletion
If a disk is unplugged and then the user tries to delete a checkpoint
the code would try to use NULL node name as it was not checked.
Fix this by fetching the whole disk definition object and verifying it
was found.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 03a8321135..326707e098 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -125,12 +125,15 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
for (i = 0; i < chkdef->ndisks; i++) {
virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];
- const char *node;
+ virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def,
chkdisk->name);
+
+ /* domdisk can be missing e.g. when it was unplugged */
+ if (!domdisk)
+ continue;
if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
- node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
/* If any ancestor checkpoint has a bitmap for the same
* disk, then this bitmap must be merged to the
* ancestor. */
@@ -153,20 +156,28 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
if (!(arr = virJSONValueNewArray()))
return -1;
- if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, node,
chkdisk->bitmap) < 0)
+ if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+
domdisk->src->nodeformat,
+ chkdisk->bitmap)
< 0)
return -1;
if (chkcurrent) {
- if (qemuMonitorTransactionBitmapEnable(actions, node,
disk2->bitmap) < 0)
+ if (qemuMonitorTransactionBitmapEnable(actions,
+
domdisk->src->nodeformat,
+ disk2->bitmap) < 0)
return -1;
}
- if (qemuMonitorTransactionBitmapMerge(actions, node, disk2->bitmap,
&arr) < 0)
+ if (qemuMonitorTransactionBitmapMerge(actions,
+ domdisk->src->nodeformat,
+ disk2->bitmap, &arr) <
0)
return -1;
}
}
- if (qemuMonitorTransactionBitmapRemove(actions, node, chkdisk->bitmap) <
0)
+ if (qemuMonitorTransactionBitmapRemove(actions,
+ domdisk->src->nodeformat,
+ chkdisk->bitmap) < 0)
return -1;
}
--
2.24.1
12:47 p.m.
New subject: [RFC PATCH 06/16] qemu: checkpoint: tolerate missing disks on checkpoint deletion
On 1/9/20 12:21 PM, Peter Krempa wrote:
If a disk is unplugged and then the user tries to delete a
checkpoint
the code would try to use NULL node name as it was not checked.
Fix this by fetching the whole disk definition object and verifying it
was found.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
Reviewed-by: Eric Blake <eblake(a)redhat.com>
But it also makes me wonder if the act of hot-unplug should update the
definition of existing checkpoints. (Doesn't stop this patch from being
useful as-is, but may point to further design work and future patches)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
3:11 a.m.
New subject: [RFC PATCH 06/16] qemu: checkpoint: tolerate missing disks on checkpoint deletion
On Fri, Jan 24, 2020 at 12:47:41 -0600, Eric Blake wrote:
On 1/9/20 12:21 PM, Peter Krempa wrote:
> If a disk is unplugged and then the user tries to delete a checkpoint
> the code would try to use NULL node name as it was not checked.
>
> Fix this by fetching the whole disk definition object and verifying it
> was found.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_checkpoint.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
Reviewed-by: Eric Blake <eblake(a)redhat.com>
But it also makes me wonder if the act of hot-unplug should update the
definition of existing checkpoints. (Doesn't stop this patch from being
useful as-is, but may point to further design work and future patches)
It can't since we chose to not version checkpoints on snapshots. Thus if
you've taken a checkpoint and then a snapshot and want to detach the
disk, reverting to the checkpoint will add the disk back and the
checkpoint must stay valid. As the checkpoint is not versioned we
wouldn't have the data to add it back.
12:21 p.m.
New subject: [libvirt] [RFC PATCH 07/16] qemu: domain: Remove unused qemuDomainDiskNodeFormatLookup
The function has no users now and there's no need for it as the common
pattern is to look up the whole disk object anyways.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 14 --------------
src/qemu/qemu_domain.h | 3 ---
2 files changed, 17 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..4966f5ef36 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -12048,20 +12048,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
}
-/* Return the format node name for a given disk of an online guest */
-const char *
-qemuDomainDiskNodeFormatLookup(virDomainObjPtr vm,
- const char *disk)
-{
- size_t i;
-
- for (i = 0; i < vm->def->ndisks; i++) {
- if (STREQ(vm->def->disks[i]->dst, disk))
- return vm->def->disks[i]->src->nodeformat;
- }
- return NULL;
-}
-
bool
qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index c6afc484f6..4e2ddf6880 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -864,9 +864,6 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
virDomainDiskDefPtr orig_disk);
-const char *qemuDomainDiskNodeFormatLookup(virDomainObjPtr vm,
- const char *disk);
-
void qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm,
virStorageSourcePtr src,
--
2.24.1
12:49 p.m.
New subject: [RFC PATCH 07/16] qemu: domain: Remove unused qemuDomainDiskNodeFormatLookup
On 1/9/20 12:21 PM, Peter Krempa wrote:
The function has no users now and there's no need for it as the
common
pattern is to look up the whole disk object anyways.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 14 --------------
src/qemu/qemu_domain.h | 3 ---
2 files changed, 17 deletions(-)
Code deletion is easy to review :)
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 08/16] qemu: checkpoint: Introduce helper to find checkpoint disk definition in parents
The algorithm is used in two places to find the parent checkpoint object
which contains given disk and then uses data from the disk. Additionally
the code is written in a very non-obvious way. Factor out the lookup of
the disk into a function which also simplifies the callers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 129 ++++++++++++++++++++-----------------
1 file changed, 71 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 326707e098..1100f6e744 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -104,6 +104,50 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
}
+/**
+ * qemuCheckpointFindActiveDiskInParent:
+ * @vm: domain object
+ * @from: starting moment object
+ * @diskname: name (target) of the disk to find
+ *
+ * Find the first checkpoint starting from @from continuing through parents
+ * of the checkpoint which describes disk @diskname. Return the pointer to the
+ * definition of the disk.
+ */
+static virDomainCheckpointDiskDef *
+qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,
+ virDomainMomentObjPtr from,
+ const char *diskname)
+{
+ virDomainMomentObjPtr parent = from;
+ virDomainCheckpointDefPtr parentdef = NULL;
+ size_t i;
+
+ while (parent) {
+ parentdef = virDomainCheckpointObjGetDef(parent);
+
+ for (i = 0; i < parentdef->ndisks; i++) {
+ virDomainCheckpointDiskDef *chkdisk = &parentdef->disks[i];
+
+ if (STRNEQ(chkdisk->name, diskname))
+ continue;
+
+ /* currently inspected checkpoint doesn't describe the disk,
+ * continue into parent checkpoint */
+ if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+ break;
+
+ return chkdisk;
+ }
+
+ parent = virDomainCheckpointFindByName(vm->checkpoints,
+ parentdef->parent.parent_name);
+ }
+
+ return NULL;
+}
+
+
static int
qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
virDomainCheckpointDefPtr chkdef,
@@ -112,13 +156,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverPtr driver = priv->driver;
- virDomainMomentObjPtr moment;
- virDomainCheckpointDefPtr parentdef = NULL;
- bool search_parents;
int rc;
g_autoptr(virJSONValue) actions = NULL;
size_t i;
- size_t j;
if (!(actions = virJSONValueNewArray()))
return -1;
@@ -126,6 +166,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
for (i = 0; i < chkdef->ndisks; i++) {
virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];
virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def,
chkdisk->name);
+ virDomainCheckpointDiskDef *parentchkdisk = NULL;
/* domdisk can be missing e.g. when it was unplugged */
if (!domdisk)
@@ -137,42 +178,28 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
/* If any ancestor checkpoint has a bitmap for the same
* disk, then this bitmap must be merged to the
* ancestor. */
- search_parents = true;
- for (moment = parent;
- search_parents && moment;
- moment = virDomainCheckpointFindByName(vm->checkpoints,
- parentdef->parent.parent_name)) {
- parentdef = virDomainCheckpointObjGetDef(moment);
- for (j = 0; j < parentdef->ndisks; j++) {
- virDomainCheckpointDiskDef *disk2;
- g_autoptr(virJSONValue) arr = NULL;
-
- disk2 = &parentdef->disks[j];
- if (STRNEQ(chkdisk->name, disk2->name) ||
- disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
- continue;
- search_parents = false;
-
- if (!(arr = virJSONValueNewArray()))
- return -1;
+ if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent,
chkdisk->name))) {
+ g_autoptr(virJSONValue) arr = NULL;
- if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
-
domdisk->src->nodeformat,
- chkdisk->bitmap)
< 0)
- return -1;
+ if (!(arr = virJSONValueNewArray()))
+ return -1;
- if (chkcurrent) {
- if (qemuMonitorTransactionBitmapEnable(actions,
-
domdisk->src->nodeformat,
- disk2->bitmap) < 0)
- return -1;
- }
+ if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+
domdisk->src->nodeformat,
+ chkdisk->bitmap) <
0)
+ return -1;
- if (qemuMonitorTransactionBitmapMerge(actions,
- domdisk->src->nodeformat,
- disk2->bitmap, &arr) <
0)
+ if (chkcurrent) {
+ if (qemuMonitorTransactionBitmapEnable(actions,
+ domdisk->src->nodeformat,
+ parentchkdisk->bitmap) < 0)
return -1;
}
+
+ if (qemuMonitorTransactionBitmapMerge(actions,
+ domdisk->src->nodeformat,
+ parentchkdisk->bitmap, &arr)
< 0)
+ return -1;
}
if (qemuMonitorTransactionBitmapRemove(actions,
@@ -324,14 +351,12 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
virDomainMomentObjPtr old_current,
virDomainCheckpointDefPtr def)
{
- size_t i, j;
- virDomainCheckpointDefPtr olddef;
- virDomainMomentObjPtr parent;
- bool search_parents;
+ size_t i;
for (i = 0; i < def->ndisks; i++) {
virDomainCheckpointDiskDef *chkdisk = &def->disks[i];
virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def,
chkdisk->name);
+ virDomainCheckpointDiskDef *parentchkdisk = NULL;
/* checkpoint definition validator mandates that the corresponding
* domdisk should exist */
@@ -351,25 +376,13 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
* iteration; but it is also possible to have to search
* further than the immediate parent to find another
* checkpoint with a bitmap on the same disk. */
- search_parents = true;
- for (parent = old_current; search_parents && parent;
- parent = virDomainCheckpointFindByName(vm->checkpoints,
- olddef->parent.parent_name)) {
- olddef = virDomainCheckpointObjGetDef(parent);
- for (j = 0; j < olddef->ndisks; j++) {
- virDomainCheckpointDiskDef *disk2;
-
- disk2 = &olddef->disks[j];
- if (STRNEQ(chkdisk->name, disk2->name) ||
- disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
- continue;
- if (qemuMonitorTransactionBitmapDisable(actions,
- domdisk->src->nodeformat,
- disk2->bitmap) < 0)
- return -1;
- search_parents = false;
- break;
- }
+ if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, old_current,
+ chkdisk->name))) {
+
+ if (qemuMonitorTransactionBitmapDisable(actions,
+ domdisk->src->nodeformat,
+ parentchkdisk->bitmap) < 0)
+ return -1;
}
}
return 0;
--
2.24.1
12:55 p.m.
New subject: [RFC PATCH 08/16] qemu: checkpoint: Introduce helper to find checkpoint disk definition in parents
On 1/9/20 12:21 PM, Peter Krempa wrote:
The algorithm is used in two places to find the parent checkpoint
object
which contains given disk and then uses data from the disk. Additionally
the code is written in a very non-obvious way. Factor out the lookup of
the disk into a function which also simplifies the callers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 129 ++++++++++++++++++++-----------------
1 file changed, 71 insertions(+), 58 deletions(-)
Thank you. I noticed the repetition when I was preparing my v10 of
incremental backup, but did not manage to factor it out cleanly like this.
+/**
+ * qemuCheckpointFindActiveDiskInParent:
+ * @vm: domain object
+ * @from: starting moment object
+ * @diskname: name (target) of the disk to find
+ *
+ * Find the first checkpoint starting from @from continuing through parents
+ * of the checkpoint which describes disk @diskname. Return the pointer to the
+ * definition of the disk.
+ */
+static virDomainCheckpointDiskDef *
+qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,
+ virDomainMomentObjPtr from,
+ const char *diskname)
+{
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 09/16] qemu: checkpoint: Extract calculation of bitmap merging for checkpoint deletion
This will allow some testing before refactoring.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 72 ++++++++++++++++++++++++--------------
src/qemu/qemu_checkpoint.h | 7 ++++
2 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 1100f6e744..e75cdd0458 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -148,6 +148,46 @@ qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,
}
+int
+qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
+ const char *delbitmap,
+ const char *parentbitmap,
+ bool chkcurrent,
+ virJSONValuePtr actions)
+{
+ if (parentbitmap) {
+ g_autoptr(virJSONValue) arr = NULL;
+
+ if (!(arr = virJSONValueNewArray()))
+ return -1;
+
+ if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+ src->nodeformat,
+ delbitmap) < 0)
+ return -1;
+
+ if (chkcurrent) {
+ if (qemuMonitorTransactionBitmapEnable(actions,
+ src->nodeformat,
+ parentbitmap) < 0)
+ return -1;
+ }
+
+ if (qemuMonitorTransactionBitmapMerge(actions,
+ src->nodeformat,
+ parentbitmap, &arr) < 0)
+ return -1;
+ }
+
+ if (qemuMonitorTransactionBitmapRemove(actions,
+ src->nodeformat,
+ delbitmap) < 0)
+ return -1;
+
+ return 0;
+}
+
+
static int
qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
virDomainCheckpointDefPtr chkdef,
@@ -167,6 +207,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];
virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def,
chkdisk->name);
virDomainCheckpointDiskDef *parentchkdisk = NULL;
+ const char *parentbitmap = NULL;
/* domdisk can be missing e.g. when it was unplugged */
if (!domdisk)
@@ -178,33 +219,12 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
/* If any ancestor checkpoint has a bitmap for the same
* disk, then this bitmap must be merged to the
* ancestor. */
- if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent,
chkdisk->name))) {
- g_autoptr(virJSONValue) arr = NULL;
-
- if (!(arr = virJSONValueNewArray()))
- return -1;
-
- if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
-
domdisk->src->nodeformat,
- chkdisk->bitmap) <
0)
- return -1;
-
- if (chkcurrent) {
- if (qemuMonitorTransactionBitmapEnable(actions,
- domdisk->src->nodeformat,
- parentchkdisk->bitmap) < 0)
- return -1;
- }
-
- if (qemuMonitorTransactionBitmapMerge(actions,
- domdisk->src->nodeformat,
- parentchkdisk->bitmap, &arr)
< 0)
- return -1;
- }
+ if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent,
+ chkdisk->name)))
+ parentbitmap = parentchkdisk->name;
- if (qemuMonitorTransactionBitmapRemove(actions,
- domdisk->src->nodeformat,
- chkdisk->bitmap) < 0)
+ if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, chkdisk->bitmap,
+ parentbitmap, chkcurrent, actions) < 0)
return -1;
}
diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
index eb85611ea6..85fd453d50 100644
--- a/src/qemu/qemu_checkpoint.h
+++ b/src/qemu/qemu_checkpoint.h
@@ -71,3 +71,10 @@ qemuCheckpointCreateFinalize(virQEMUDriverPtr driver,
void
qemuCheckpointRollbackMetadata(virDomainObjPtr vm,
virDomainMomentObjPtr chk);
+
+int
+qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
+ const char *delbitmap,
+ const char *parentbitmap,
+ bool chkcurrent,
+ virJSONValuePtr actions);
--
2.24.1
12:56 p.m.
New subject: [RFC PATCH 09/16] qemu: checkpoint: Extract calculation of bitmap merging for checkpoint deletion
On 1/9/20 12:21 PM, Peter Krempa wrote:
This will allow some testing before refactoring.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 72 ++++++++++++++++++++++++--------------
src/qemu/qemu_checkpoint.h | 7 ++++
2 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 10/16] tests: qemublock: Add test for checkpoint deletion bitmap merge
Add test infrastructure and a basic test for bitmap deletion.
Signed-off-by: Peter Krempa <pkrempa(a)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 3e9edb85f0..3ed2486ad2 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"
@@ -696,6 +697,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)
{
@@ -705,6 +750,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;
@@ -941,6 +987,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
12:55 p.m.
New subject: [RFC PATCH 10/16] tests: qemublock: Add test for checkpoint deletion bitmap merge
On 1/9/20 12:21 PM, Peter Krempa wrote:
Add test infrastructure and a basic test for bitmap deletion.
Signed-off-by: Peter Krempa <pkrempa(a)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
+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) {
Are there patches planned later on to change this? Or is this comment
going to be an instance of technical debt for an unknown length of time?
At any rate, the test looks sane enough;
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1:37 a.m.
New subject: [RFC PATCH 10/16] tests: qemublock: Add test for checkpoint deletion bitmap merge
On Mon, Jan 27, 2020 at 12:55:02 -0600, Eric Blake wrote:
On 1/9/20 12:21 PM, Peter Krempa wrote:
> Add test infrastructure and a basic test for bitmap deletion.
>
> Signed-off-by: Peter Krempa <pkrempa(a)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
>
> +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) {
Are there patches planned later on to change this? Or is this comment going
to be an instance of technical debt for an unknown length of time?
Patch 13 modifies qemuCheckpointDiscardDiskBitmaps to work accross the
backing chain and after that modification it doesn't require the
parameter any more.
12:21 p.m.
New subject: [libvirt] [RFC PATCH 11/16] tests: qemublock: Add few more test cases for checkpoint deletion
Add all intermediate steps and deletion of the current checkpoint on a
flat (single-image) disk image.
Signed-off-by: Peter Krempa <pkrempa(a)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 3ed2486ad2..203e0f70b1 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -999,6 +999,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
8:15 a.m.
New subject: [RFC PATCH 11/16] tests: qemublock: Add few more test cases for checkpoint deletion
On 1/9/20 12:21 PM, Peter Krempa wrote:
Add all intermediate steps and deletion of the current checkpoint on
a
flat (single-image) disk image.
Signed-off-by: Peter Krempa <pkrempa(a)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
+++
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"
+ }
+ }
+]
Creates a rather large transaction (3 items, of which one itself takes
an array of bitmaps), but that's par for the course. The test looks good.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 12/16] tests: qemublock: Add synthetic snapshot+checkpoint test data
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(a)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 203e0f70b1..8a3f7da029 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -966,6 +966,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
8:37 a.m.
New subject: [RFC PATCH 12/16] tests: qemublock: Add synthetic snapshot+checkpoint test data
On 1/9/20 12:21 PM, Peter Krempa wrote:
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(a)redhat.com>
---
tests/qemublocktest.c | 1 +
.../snapshots-synthetic-checkpoint.json | 827 ++++++++++++++++++
Hmm - proof why we want qemu to add the optional flat parameter to avoid
the O(n^2) growth of output as backing chains get longer.
.../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
The test itself looks sane.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:21 p.m.
New subject: [libvirt] [RFC PATCH 13/16] qemu: checkpoint: Introduce support for deleting checkpoints accross snapshots
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(a)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 8a3f7da029..293757b8d4 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -704,6 +704,7 @@ struct testQemuCheckpointDeleteMergeData {
virStorageSourcePtr chain;
const char *deletebitmap;
const char *parentbitmap;
+ const char *nodedatafile;
};
@@ -714,22 +715,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;
}
@@ -988,22 +997,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
12:21 p.m.
New subject: [libvirt] [RFC PATCH 14/16] tests: qemublock: Add checkpoint deletion test for deep backing chain
Add test cases for merging various pairs of bitmaps when snapshots were
created together with checkpoints.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemublocktest.c | 6 +++
.../snapshots-current-out.json | 29 +++++++++++
.../snapshots-intermediate1-out.json | 22 ++++++++
.../snapshots-intermediate2-out.json | 52 +++++++++++++++++++
.../snapshots-intermediate3-out.json | 52 +++++++++++++++++++
.../snapshots-noparent-out.json | 23 ++++++++
6 files changed, 184 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 293757b8d4..b8a052b19b 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1015,6 +1015,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..ca5627269c
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
@@ -0,0 +1,52 @@
+[
+ {
+ "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-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..ee7ae97fdb
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
@@ -0,0 +1,52 @@
+[
+ {
+ "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-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
12:21 p.m.
New subject: [libvirt] [RFC PATCH 15/16] tests: qemublock: Add checkpoint deletion tests for some special cases
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(a)redhat.com>
---
tests/qemublocktest.c | 7 +++
.../snapshots-intermediate2-out.json | 7 +++
.../snapshots-intermediate3-out.json | 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 ++++++++
8 files changed, 193 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 b8a052b19b..c2c7507e20 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1021,6 +1021,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-intermediate2-out.json
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
index ca5627269c..4da21a9df7 100644
--- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
@@ -29,6 +29,13 @@
"name": "c"
}
},
+ {
+ "type": "block-dirty-bitmap-enable",
+ "data": {
+ "node": "libvirt-3-format",
+ "name": "b"
+ }
+ },
{
"type": "block-dirty-bitmap-merge",
"data": {
diff --git a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
index ee7ae97fdb..dc87dd60b8 100644
--- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
@@ -29,6 +29,13 @@
"name": "d"
}
},
+ {
+ "type": "block-dirty-bitmap-enable",
+ "data": {
+ "node": "libvirt-2-format",
+ "name": "c"
+ }
+ },
{
"type": "block-dirty-bitmap-merge",
"data": {
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
12:21 p.m.
New subject: [libvirt] [RFC PATCH 16/16] qemu: checkpoint: Track and relabel images for bitmap merging
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(a)redhat.com>
---
src/qemu/qemu_checkpoint.c | 33 ++++++++++++++++---
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, 69 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 087a740cf8..13bd6653b7 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;
}
@@ -253,6 +257,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
int rc;
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 c2c7507e20..a368e6a74f 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -717,6 +717,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);
@@ -738,14 +741,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
10:49 a.m.
New subject: [RFC PATCH 00/16] qemu: checkpoint: Add support for deleting checkpoints accross snapshots
On 1/9/20 12:21 PM, Peter Krempa wrote:
Posted as RFC because qemu probably doesn't reopen backing images
for
bitmap operations resulting in:
$ virsh checkpoint-delete vm --checkpointname a
error: Failed to delete checkpoint a
error: internal error: unable to execute QEMU command 'transaction': Bitmap
'a' is readonly and cannot be modified
Unfortunately this can't be done manually because 'blockdev-reopen' is
still experimental.
In the meantime, with existing qemu, can we still at least teach libvirt
to delete its metadata, and ignore unknown bitmaps leftover in qcow2
images that we were unable to delete because of the qemu limitation on
read-only backing files? Yes, your qcow2 files will get bloated with
dead bitmaps, but that's not going to interfere with guest operations or
with future backups that use the bitmaps still tracked by libvirt. And
it matches the failure scenario where an attempt at an incremental
backup fails due to qemu crashing: the active bitmap is corrupt, so all
older bitmaps are no longer usable, and your only recourse is to resort
to full backup instead of incremental backup; but full backup is still
worthwhile, and ignoring the now-useless bitmaps from the earlier backup
points doesn't interfere with that.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1763
days inactive
1782
days old
33 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa