Once a checkpoint has been created, it is desirable to estimate the
size of the disk delta that is represented between the checkpoint and
the current operation. To do this, we have to scrape information out
of QMP query-block on a request from the user.
---
 src/qemu/qemu_monitor.h      |  4 ++
 src/qemu/qemu_monitor_json.h |  3 ++
 src/qemu/qemu_driver.c       | 56 +++++++++++++++++++++++++-
 src/qemu/qemu_monitor.c      | 11 ++++++
 src/qemu/qemu_monitor_json.c | 76 ++++++++++++++++++++++++++++++++++++
 5 files changed, 149 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index a19d6069c6..ac9726af39 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -25,6 +25,7 @@
 # include "internal.h"

 # include "domain_conf.h"
+# include "checkpoint_conf.h"
 # include "virbitmap.h"
 # include "virhash.h"
 # include "virjson.h"
@@ -630,6 +631,9 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
 int qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
                                                 virHashTablePtr stats)
     ATTRIBUTE_NONNULL(2);
+int qemuMonitorUpdateCheckpointSize(qemuMonitorPtr mon,
+                                    virDomainCheckpointDefPtr chk)
+    ATTRIBUTE_NONNULL(2);

 int qemuMonitorBlockResize(qemuMonitorPtr mon,
                            const char *device,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 2a881fbddd..aa68b35003 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -98,6 +98,9 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
                                const char *nodename,
                                unsigned long long size);

+int qemuMonitorJSONUpdateCheckpointSize(qemuMonitorPtr mon,
+                                        virDomainCheckpointDefPtr chk);
+
 int qemuMonitorJSONSetPassword(qemuMonitorPtr mon,
                                const char *protocol,
                                const char *password,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0cdb2dd1e2..585a0203eb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17384,10 +17384,14 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint,
     virDomainObjPtr vm = NULL;
     char *xml = NULL;
     virDomainMomentObjPtr chk = NULL;
+    qemuDomainObjPrivatePtr priv;
+    int rc;
+    size_t i;
     virDomainCheckpointDefPtr chkdef;

     virCheckFlags(VIR_DOMAIN_CHECKPOINT_XML_SECURE |
-                  VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN, NULL);
+                  VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN |
+                  VIR_DOMAIN_CHECKPOINT_XML_SIZE, NULL);

     if (!(vm = qemuDomObjFromCheckpoint(checkpoint)))
         return NULL;
@@ -17399,9 +17403,59 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint,
         goto cleanup;
     chkdef = virDomainCheckpointObjGetDef(chk);

+    if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE) {
+        /* TODO: for non-current checkpoint, this requires a QMP sequence per
+           disk, since the stat of one bitmap in isolation is too low,
+           and merely adding bitmap sizes may be too high:
+             block-dirty-bitmap-create tmp
+             for each bitmap from checkpoint to current:
+               add bitmap to src_list
+             block-dirty-bitmap-merge dst=tmp src_list
+             query-block and read tmp size
+             block-dirty-bitmap-remove tmp
+           So for now, go with simpler query-blocks only for current.
+        */
+        if (virDomainCheckpointGetCurrent(vm->checkpoints) != chk) {
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                           _("cannot compute size for non-current checkpoint '%s'"),
+                           checkpoint->name);
+            goto cleanup;
+        }
+
+        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+            goto cleanup;
+
+        if (virDomainObjCheckActive(vm) < 0)
+            goto endjob;
+
+        if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
+            goto endjob;
+
+        /* TODO: Shouldn't need to recompute node names. */
+        for (i = 0; i < chkdef->ndisks; i++) {
+            virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
+
+            if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+                continue;
+            VIR_FREE(chk->def->dom->disks[disk->idx]->src->nodeformat);
+            if (VIR_STRDUP(chk->def->dom->disks[disk->idx]->src->nodeformat,
+                           qemuBlockNodeLookup(vm, disk->name)) < 0)
+                goto endjob;
+        }
+
+        priv = vm->privateData;
+        qemuDomainObjEnterMonitor(driver, vm);
+        rc = qemuMonitorUpdateCheckpointSize(priv->mon, chkdef);
+        if (qemuDomainObjExitMonitor(driver, vm) < 0)
+            goto endjob;
+        if (rc < 0)
+            goto endjob;
+    }
+
     xml = virDomainCheckpointDefFormat(chkdef, driver->caps, driver->xmlopt,
                                        flags);

+ endjob:
     if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE)
         qemuDomainObjEndJob(driver, vm);

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d9d076633d..ee3dce87ae 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2342,6 +2342,17 @@ qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
     return qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(mon, stats);
 }

+/* Updates "chk" to fill in size of the associated bitmap */
+int qemuMonitorUpdateCheckpointSize(qemuMonitorPtr mon,
+                                    virDomainCheckpointDefPtr chk)
+{
+    VIR_DEBUG("chk=%p", chk);
+
+    QEMU_CHECK_MONITOR(mon);
+
+    return qemuMonitorJSONUpdateCheckpointSize(mon, chk);
+}
+
 int
 qemuMonitorBlockResize(qemuMonitorPtr mon,
                        const char *device,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2dcd65d86f..4c232105c2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2760,6 +2760,82 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
     return ret;
 }

+int qemuMonitorJSONUpdateCheckpointSize(qemuMonitorPtr mon,
+                                        virDomainCheckpointDefPtr chk)
+{
+    int ret = -1;
+    size_t i, j;
+    virJSONValuePtr devices;
+
+    if (!(devices = qemuMonitorJSONQueryBlock(mon)))
+        return -1;
+
+    for (i = 0; i < virJSONValueArraySize(devices); i++) {
+        virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
+        virJSONValuePtr inserted;
+        virJSONValuePtr bitmaps = NULL;
+        const char *node;
+        virDomainCheckpointDiskDefPtr disk;
+
+        if (!(dev = qemuMonitorJSONGetBlockDev(devices, i)))
+            goto cleanup;
+
+        if (!(inserted = virJSONValueObjectGetObject(dev, "inserted")))
+            continue;
+        if (!(node = virJSONValueObjectGetString(inserted, "node-name"))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("query-block device entry was not in expected format"));
+            goto cleanup;
+        }
+
+        for (j = 0; j < chk->ndisks; j++) {
+            disk = &chk->disks[j];
+            if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+                continue;
+            if (STREQ(chk->parent.dom->disks[disk->idx]->src->nodeformat, node))
+                break;
+        }
+        if (j == chk->ndisks) {
+            VIR_DEBUG("query-block did not find node %s", node);
+            continue;
+        }
+        if (!(bitmaps = virJSONValueObjectGetArray(dev, "dirty-bitmaps"))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("disk %s dirty bitmaps missing"), disk->name);
+            goto cleanup;
+        }
+        for (j = 0; j < virJSONValueArraySize(bitmaps); j++) {
+            virJSONValuePtr map = virJSONValueArrayGet(bitmaps, j);
+            const char *name;
+
+            if (!(name = virJSONValueObjectGetString(map, "name"))) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("dirty bitmaps entry was not in expected format"));
+                goto cleanup;
+            }
+            if (STRNEQ(name, disk->bitmap))
+                continue;
+            if (virJSONValueObjectGetNumberUlong(map, "count", &disk->size) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("invalid bitmap count"));
+                goto cleanup;
+            }
+            break;
+        }
+        if (j == virJSONValueArraySize(bitmaps)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("disk %s dirty bitmap info missing"), disk->name);
+            goto cleanup;
+        }
+    }
+
+    ret = 0;
+
+ cleanup:
+    virJSONValueFree(devices);
+    return ret;
+}
+

 int qemuMonitorJSONSetPassword(qemuMonitorPtr mon,
                                const char *protocol,
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Jun 18, 2019 at 22:47:54 -0500, Eric Blake wrote:
> Once a checkpoint has been created, it is desirable to estimate the
> size of the disk delta that is represented between the checkpoint and
> the current operation. To do this, we have to scrape information out
> of QMP query-block on a request from the user.
> ---
>  src/qemu/qemu_monitor.h      |  4 ++
>  src/qemu/qemu_monitor_json.h |  3 ++
>  src/qemu/qemu_driver.c       | 56 +++++++++++++++++++++++++-
>  src/qemu/qemu_monitor.c      | 11 ++++++
>  src/qemu/qemu_monitor_json.c | 76 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 149 insertions(+), 1 deletion(-)

[...]

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0cdb2dd1e2..585a0203eb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

[....]

> @@ -17399,9 +17403,59 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint,
>          goto cleanup;
>      chkdef = virDomainCheckpointObjGetDef(chk);
> 
> +    if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE) {
> +        /* TODO: for non-current checkpoint, this requires a QMP sequence per
> +           disk, since the stat of one bitmap in isolation is too low,
> +           and merely adding bitmap sizes may be too high:
> +             block-dirty-bitmap-create tmp
> +             for each bitmap from checkpoint to current:
> +               add bitmap to src_list
> +             block-dirty-bitmap-merge dst=tmp src_list
> +             query-block and read tmp size
> +             block-dirty-bitmap-remove tmp
> +           So for now, go with simpler query-blocks only for current.
> +        */
> +        if (virDomainCheckpointGetCurrent(vm->checkpoints) != chk) {
> +            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                           _("cannot compute size for non-current checkpoint '%s'"),
> +                           checkpoint->name);
> +            goto cleanup;
> +        }
> +
> +        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +            goto cleanup;
> +
> +        if (virDomainObjCheckActive(vm) < 0)
> +            goto endjob;
> +
> +        if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +            goto endjob;

Again, I pointed out multiple times that this should not be done, or at
least only in non-blockdev case.

> +
> +        /* TODO: Shouldn't need to recompute node names. */

Well, they are not meant to be stable so if you want to match it with
the saved definition it may become fun.

Also after a snapshot the checkpoint def and VM def will differ same as
the positioning of the nodenames.

This code does not seem to take it into account in any way.

And this is getting into bigger amount of technical debt which will need
to be addressed if we want to allow checkpoints and snapshots together.

> +        for (i = 0; i < chkdef->ndisks; i++) {
> +            virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
> +
> +            if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> +                continue;
> +            VIR_FREE(chk->def->dom->disks[disk->idx]->src->nodeformat);


> +            if (VIR_STRDUP(chk->def->dom->disks[disk->idx]->src->nodeformat,
> +                           qemuBlockNodeLookup(vm, disk->name)) < 0)
> +                goto endjob;
> +        }
> +
> +        priv = vm->privateData;
> +        qemuDomainObjEnterMonitor(driver, vm);
> +        rc = qemuMonitorUpdateCheckpointSize(priv->mon, chkdef);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)

[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 2dcd65d86f..4c232105c2 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2760,6 +2760,82 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
>      return ret;
>  }
> 
> +int qemuMonitorJSONUpdateCheckpointSize(qemuMonitorPtr mon,
> +                                        virDomainCheckpointDefPtr chk)
> +{
> +    int ret = -1;
> +    size_t i, j;
> +    virJSONValuePtr devices;
> +
> +    if (!(devices = qemuMonitorJSONQueryBlock(mon)))
> +        return -1;

Isn't this data available in 'query-named-block-nodes'?

> +
> +    for (i = 0; i < virJSONValueArraySize(devices); i++) {
> +        virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
> +        virJSONValuePtr inserted;
> +        virJSONValuePtr bitmaps = NULL;
> +        const char *node;
> +        virDomainCheckpointDiskDefPtr disk;
> +
> +        if (!(dev = qemuMonitorJSONGetBlockDev(devices, i)))
> +            goto cleanup;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list