[v2 PATCH] QEMU: allow update more disk properties

jshen28 posted 1 patch 1 year, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230221102129.14830-1-yshxxsjt715@163.com
src/qemu/qemu_block.c  | 69 ++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_block.h  |  9 ++++++
src/qemu/qemu_domain.c | 30 ++++++++++++++++--
src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++
4 files changed, 147 insertions(+), 2 deletions(-)
[v2 PATCH] QEMU: allow update more disk properties
Posted by jshen28 1 year, 2 months ago
From: ushen <yshxxsjt715@gmail.com>

QEMU has provided QMP blockdev-reopen to allow reconfigure some
driver properties while virtual machine is running. Such properties
are including cachmode, detect-zeroes and discard. This PS modifies
update-device a little bit to allow updating those properties.

Some cachemode types could be switched from using blockdev-reopen
without affect frontend device prameters. This PS will
compare the writeback flag between new and origin domain disk
object and continue blockdev reopen if writeback flag is the same.

Signed-off-by: shenjiatong <yshxxsjt715@gmail.com>
---
 src/qemu/qemu_block.c  | 69 ++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_block.h  |  9 ++++++
 src/qemu/qemu_domain.c | 30 ++++++++++++++++--
 src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 5e700eff99..3c45d8d7c7 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2970,6 +2970,38 @@ qemuBlockReopenFormatMon(qemuMonitor *mon,
     return 0;
 }
 
+int
+qemuBlockReopenMon(qemuMonitor *mon,
+                      virStorageSource *src)
+{
+    g_autoptr(virJSONValue) reopenprops = NULL;
+    g_autoptr(virJSONValue) srcprops = NULL;
+    g_autoptr(virJSONValue) srcstorprops = NULL;
+    g_autoptr(virJSONValue) reopenoptions = virJSONValueNewArray();
+
+    if (!(srcprops = qemuBlockStorageSourceGetBlockdevProps(src, src->backingStore)))
+        return -1;
+
+    if (!(srcstorprops = qemuBlockStorageSourceGetBackendProps(src, 0)))
+        return -1;
+
+    if (virJSONValueArrayAppend(reopenoptions, &srcprops) < 0)
+        return -1;
+
+    if (virJSONValueArrayAppend(reopenoptions, &srcstorprops) < 0)
+        return -1;
+
+    if (virJSONValueObjectAdd(&reopenprops,
+                              "a:options", &reopenoptions,
+                              NULL) < 0)
+        return -1;
+
+    if (qemuMonitorBlockdevReopen(mon, &reopenprops) < 0)
+        return -1;
+
+    return 0;
+}
+
 
 /**
  * qemuBlockReopenFormat:
@@ -3010,6 +3042,43 @@ qemuBlockReopenFormat(virDomainObj *vm,
 }
 
 
+/**
+ * qemuBlockReopen:
+ * @vm: domain object
+ * @src: storage source to reopen
+ * @asyncJob: qemu async job type
+ *
+ * Same as qemuBlockReopenFormat, but both reopens storage and format node
+ */
+int
+qemuBlockReopen(virDomainObj *vm,
+                virStorageSource *src,
+                virDomainAsyncJob asyncJob)
+{
+    qemuDomainObjPrivate *priv = vm->privateData;
+    int rc;
+
+    /* If we are lacking the object here, qemu might have opened an image with
+     * a node name unknown to us */
+    if (!src->backingStore) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("can't reopen image with unknown presence of backing store"));
+        return -1;
+    }
+
+    if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
+        return -1;
+
+    rc = qemuBlockReopenMon(priv->mon, src);
+
+    qemuDomainObjExitMonitor(vm);
+    if (rc < 0)
+        return -1;
+
+    return 0;
+}
+
+
 /**
  * qemuBlockReopenReadWrite:
  * @vm: domain object
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 5a61a19da2..28f9b6cc61 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -245,6 +245,15 @@ int
 qemuBlockReopenFormatMon(qemuMonitor *mon,
                          virStorageSource *src);
 
+
+int
+qemuBlockReopenMon(qemuMonitor *mon, virStorageSource *src);
+
+int
+qemuBlockReopen(virDomainObj *vm,
+                virStorageSource *src,
+                virDomainAsyncJob asyncJob);
+
 int
 qemuBlockReopenReadWrite(virDomainObj *vm,
                          virStorageSource *src,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e9bc0f375d..d5d768c9b2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8129,6 +8129,28 @@ qemuDomainStorageSourceAccessAllow(virQEMUDriver *driver,
 }
 
 
+static bool
+qemuCacheModeChangeSupported(virDomainDiskDef *disk,
+                             virDomainDiskDef *orig_disk)
+{
+    bool new_wt, orig_wt;
+
+    if (qemuDomainDiskCachemodeFlags(disk->cachemode, &new_wt, NULL, NULL) < 0) {
+        return false;
+    }
+
+    if (qemuDomainDiskCachemodeFlags(orig_disk->cachemode, &orig_wt, NULL, NULL) < 0) {
+        return false;
+    }
+
+    if (new_wt != orig_wt) {
+        return false;
+    }
+
+    return true;
+}
+
+
 /*
  * Makes sure the @disk differs from @orig_disk only by the source
  * path and nothing else.  Fields that are being checked and the
@@ -8239,7 +8261,12 @@ qemuDomainDiskChangeSupported(virDomainDiskDef *disk,
     CHECK_STREQ_NULLABLE(product,
                          "product");
 
-    CHECK_EQ(cachemode, "cache", true);
+     if (!qemuCacheModeChangeSupported(disk, orig_disk)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+            _("cannot modify field '%s' of the disk"),
+            "cachemode");
+        return false;
+    }
     CHECK_EQ(error_policy, "error_policy", true);
     CHECK_EQ(rerror_policy, "rerror_policy", true);
     CHECK_EQ(iomode, "io", true);
@@ -8267,7 +8294,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDef *disk,
     CHECK_EQ(info.bootIndex, "boot order", true);
     CHECK_EQ(rawio, "rawio", true);
     CHECK_EQ(sgio, "sgio", true);
-    CHECK_EQ(discard, "discard", true);
     CHECK_EQ(iothread, "iothread", true);
 
     CHECK_STREQ_NULLABLE(domain_name,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6154fe9bfe..10c76a3af6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6802,6 +6802,8 @@ qemuDomainChangeDiskLive(virDomainObj *vm,
     virDomainDiskDef *orig_disk = NULL;
     virDomainStartupPolicy origStartupPolicy;
     virDomainDeviceDef oldDev = { .type = dev->type };
+    virStorageSource *n = NULL;
+    int ret, detect_zeroes, discard, cachemode;
 
     if (!(orig_disk = virDomainDiskByTarget(vm->def, disk->dst))) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -6840,8 +6842,47 @@ qemuDomainChangeDiskLive(virDomainObj *vm,
         }
 
         dev->data.disk->src = NULL;
+    } else {
+
+        // reopen disk device with more parameters
+        detect_zeroes = orig_disk->detect_zeroes;
+        discard = orig_disk->discard;
+        cachemode = orig_disk->cachemode;
+
+        if (detect_zeroes == disk->detect_zeroes &&
+            discard == disk->discard &&
+            discard == disk->cachemode) {
+            goto out;
+        }
+
+        orig_disk->detect_zeroes = disk->detect_zeroes;
+        orig_disk->discard = disk->discard;
+        orig_disk->cachemode = disk->cachemode;
+
+        for (n = orig_disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
+            if (n == orig_disk->src && detect_zeroes != disk->detect_zeroes) {
+                n->detect_zeroes = disk->detect_zeroes;
+            }
+            n->discard = disk->discard;
+            n->cachemode = disk->cachemode;
+            ret = qemuBlockReopen(vm, n, VIR_ASYNC_JOB_NONE);
+            if (ret < 0) {
+                if (n == orig_disk->src) {
+                    orig_disk->detect_zeroes = detect_zeroes;
+                    orig_disk->src->detect_zeroes = detect_zeroes;
+                    orig_disk->discard = discard;
+                    orig_disk->src->discard = discard;
+                    orig_disk->cachemode = cachemode;
+                    orig_disk->src->cachemode = cachemode;
+                    virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                                  _("failed to blockreopen disk"));
+                    return -1;
+                }
+            }
+        }
     }
 
+ out:
     /* in case when we aren't updating disk source we update startup policy here */
     orig_disk->startupPolicy = dev->data.disk->startupPolicy;
     orig_disk->snapshot = dev->data.disk->snapshot;
-- 
2.17.1