[PATCH v2 16/19] qemuDomainBlockCommit: Handle bitmaps on start of commit

Peter Krempa posted 19 patches 5 years, 11 months ago
[PATCH v2 16/19] qemuDomainBlockCommit: Handle bitmaps on start of commit
Posted by Peter Krempa 5 years, 11 months ago
On start of the commit job, we need to disable any active bitmap in the
base. Use qemuBlockBitmapsHandleCommitStart to calculate which and call
the appropriate QMP APIs. We use blockdev-reopen to make the 'base'
writable to disable the bitmaps.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 31c0f2dd91..628fe9b107 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18399,6 +18399,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
     const char *nodebase = NULL;
     bool persistjob = false;
     bool blockdev = false;
+    g_autoptr(virJSONValue) bitmapDisableActions = NULL;
+    VIR_AUTOSTRINGLIST bitmapDisableList = NULL;

     virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
                   VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
@@ -18555,8 +18557,29 @@ qemuDomainBlockCommit(virDomainPtr dom,
             goto endjob;
     }

+    if (blockdev &&
+        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN)) {
+        g_autoptr(virHashTable) blockNamedNodeData = NULL;
+        if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_NONE)))
+            goto endjob;
+
+        if (qemuBlockBitmapsHandleCommitStart(topSource, baseSource,
+                                              blockNamedNodeData,
+                                              &bitmapDisableActions,
+                                              &bitmapDisableList) < 0)
+            goto endjob;
+
+        /* if we don't have terminator on 'base' we can't reopen it */
+        if (bitmapDisableActions && !baseSource->backingStore) {
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                           _("can't handle bitmaps on unterminated backing image '%s'"),
+                           base);
+            goto endjob;
+        }
+    }
+
     if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
-                                          baseSource, NULL,
+                                          baseSource, &bitmapDisableList,
                                           flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE,
                                           flags)))
         goto endjob;
@@ -18578,6 +18601,24 @@ qemuDomainBlockCommit(virDomainPtr dom,
         if (!backingPath && top_parent &&
             !(backingPath = qemuBlockGetBackingStoreString(baseSource)))
             goto endjob;
+
+        if (bitmapDisableActions) {
+            int rc;
+
+            if (qemuBlockReopenReadWrite(vm, baseSource, QEMU_ASYNC_JOB_NONE) < 0)
+                goto endjob;
+
+            qemuDomainObjEnterMonitor(driver, vm);
+            rc = qemuMonitorTransaction(priv->mon, &bitmapDisableActions);
+            if (qemuDomainObjExitMonitor(driver, vm) < 0)
+                goto endjob;
+
+            if (qemuBlockReopenReadOnly(vm, baseSource, QEMU_ASYNC_JOB_NONE) < 0)
+                goto endjob;
+
+            if (rc < 0)
+                goto endjob;
+        }
     } else {
         device = job->name;
     }
-- 
2.24.1

Re: [PATCH v2 16/19] qemuDomainBlockCommit: Handle bitmaps on start of commit
Posted by Eric Blake 5 years, 11 months ago
On 3/11/20 7:56 AM, Peter Krempa wrote:
> On start of the commit job, we need to disable any active bitmap in the
> base. Use qemuBlockBitmapsHandleCommitStart to calculate which and call
> the appropriate QMP APIs. We use blockdev-reopen to make the 'base'
> writable to disable the bitmaps.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>   src/qemu/qemu_driver.c | 43 +++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 42 insertions(+), 1 deletion(-)
> 

Someday, it would be nice if qemu let us do this without requiring us to 
do the reopen step ourselves, but in the meantime, this looks like it 
works.  Design-wise, I'm still not convinced why we have to disable the 
bitmaps in the base prior to the commit (as the only thing a bitmap is 
good for is for cumulative merging in determining how much of an image 
to expose over a future differential backup, but that differential is 
the same whether we write bits in one or multiple bitmaps as part of the 
commit operation).  But code wise, this looks accurate.

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

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