[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v2 8/8] blockcopy: add qemu implementation of new tunables



Upstream qemu 1.4 added some drive-mirror tunables not present
when it was first introduced in 1.3.  Management apps may want
to set these in some cases (for example, without tuning
granularity down to sector size, a copy may end up occupying
more bytes than the original because an entire cluster is
copied even when only a sector within the cluster is dirty,
although tuning it down results in more CPU time to do the
copy).  I haven't personally needed to use the parameters, but
since they exist, and since the new API supports virTypedParams,
we might as well expose them.

Since the tuning parameters aren't often used, and omitted from
the QMP command when unspecified, I think it is safe to rely on
qemu 1.3 to issue an error about them being unsupported, rather
than trying to create a new capability bit in libvirt.

Meanwhile, all versions of qemu from 1.4 to 2.1 have a bug where
a bad granularity (such as non-power-of-2) gives a poor message:
error: internal error: unable to execute QEMU command 'drive-mirror': Invalid parameter 'drive-virtio-disk0'

because of abuse of QERR_INVALID_PARAMETER (which is supposed to
name the parameter that was given a bad value, rather than the
value passed to some other parameter).  I don't see that a
capability check will help, so we'll just live with it (and I'll
send a patch to get qemu 2.2 to give a nicer message).

* src/qemu/qemu_monitor.h (qemuMonitorDriveMirror): Add
parameters.
* src/qemu/qemu_monitor.c (qemuMonitorDriveMirror): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDriveMirror):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDriveMirror):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainBlockCopyCommon): Likewise.
(qemuDomainBlockRebase, qemuDomainBlockCopy): Adjust callers.
* src/qemu/qemu_migration.c (qemuMigrationDriveMirror): Likewise.
* tests/qemumonitorjsontest.c (qemuMonitorJSONDriveMirror): Likewise.

Signed-off-by: Eric Blake <eblake redhat com>
---

Maybe I need to tweak libvirt's handler to report a nicer message
if qemu reports an invalid parameter, since playing the qemu
message verbatim isn't very informative.

 src/qemu/qemu_driver.c       | 18 +++++-------------
 src/qemu/qemu_migration.c    |  2 +-
 src/qemu/qemu_monitor.c      |  8 +++++---
 src/qemu/qemu_monitor.h      |  2 ++
 src/qemu/qemu_monitor_json.c |  3 +++
 src/qemu/qemu_monitor_json.h |  2 ++
 tests/qemumonitorjsontest.c  |  2 +-
 7 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4876617..d43d257 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15281,6 +15281,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr *vmptr,
                           const char *path,
                           virStorageSourcePtr dest,
                           unsigned long bandwidth,
+                          int granularity,
+                          int buf_size,
                           unsigned int flags)
 {
     virDomainObjPtr vm = *vmptr;
@@ -15421,7 +15423,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr *vmptr,
     /* Actually start the mirroring */
     qemuDomainObjEnterMonitor(driver, vm);
     ret = qemuMonitorDriveMirror(priv->mon, device, dest->path, format,
-                                 bandwidth, flags);
+                                 bandwidth, granularity, buf_size, flags);
     virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0);
     qemuDomainObjExitMonitor(driver, vm);
     if (ret < 0) {
@@ -15498,7 +15500,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
      * and VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT map to the same value
      * as for block copy. */
     ret = qemuDomainBlockCopyCommon(&vm, dom->conn, path, dest,
-                                    bandwidth, flags);
+                                    bandwidth, 0, 0, flags);

  cleanup:
     if (vm)
@@ -15561,23 +15563,13 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml,
             buf_size = params[i].value.ui;
         }
     }
-    if (granularity) {
-        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-                       _("granularity tuning not supported yet"));
-        goto cleanup;
-    }
-    if (buf_size) {
-        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-                       _("buffer size tuning not supported yet"));
-        goto cleanup;
-    }

     if (!(dest = virDomainDiskDefSourceParse(destxml, vm->def, driver->xmlopt,
                                              VIR_DOMAIN_XML_INACTIVE)))
         goto cleanup;

     ret = qemuDomainBlockCopyCommon(&vm, dom->conn, disk, dest,
-                                    bandwidth, flags);
+                                    bandwidth, granularity, buf_size, flags);

  cleanup:
     virStorageSourceFree(dest);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9cfb77e..111f6af 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1279,7 +1279,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
                                            QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
             goto error;
         mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest,
-                                         NULL, speed, mirror_flags);
+                                         NULL, speed, 0, 0, mirror_flags);
         qemuDomainObjExitMonitor(driver, vm);

         if (mon_ret < 0)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d5ba08d..0332091 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3182,14 +3182,16 @@ int
 qemuMonitorDriveMirror(qemuMonitorPtr mon,
                        const char *device, const char *file,
                        const char *format, unsigned long bandwidth,
+                       unsigned int granularity, unsigned int buf_size,
                        unsigned int flags)
 {
     int ret = -1;
     unsigned long long speed;

     VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s, bandwidth=%ld, "
-              "flags=%x",
-              mon, device, file, NULLSTR(format), bandwidth, flags);
+              "granularity=%#x, buf_size=%#x, flags=%x",
+              mon, device, file, NULLSTR(format), bandwidth, granularity,
+              buf_size, flags);

     /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
      * limited to LLONG_MAX also for unsigned values */
@@ -3204,7 +3206,7 @@ qemuMonitorDriveMirror(qemuMonitorPtr mon,

     if (mon->json)
         ret = qemuMonitorJSONDriveMirror(mon, device, file, format, speed,
-                                         flags);
+                                         granularity, buf_size, flags);
     else
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("drive-mirror requires JSON monitor"));
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 4fd6f01..9da7ee4 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -650,6 +650,8 @@ int qemuMonitorDriveMirror(qemuMonitorPtr mon,
                            const char *file,
                            const char *format,
                            unsigned long bandwidth,
+                           unsigned int granularity,
+                           unsigned int buf_size,
                            unsigned int flags)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 int qemuMonitorDrivePivot(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 62e7d5d..dcbb693 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3397,6 +3397,7 @@ int
 qemuMonitorJSONDriveMirror(qemuMonitorPtr mon,
                            const char *device, const char *file,
                            const char *format, unsigned long long speed,
+                           unsigned int granularity, unsigned int buf_size,
                            unsigned int flags)
 {
     int ret = -1;
@@ -3409,6 +3410,8 @@ qemuMonitorJSONDriveMirror(qemuMonitorPtr mon,
                                      "s:device", device,
                                      "s:target", file,
                                      "U:speed", speed,
+                                     "z:granularity", granularity,
+                                     "z:buf-size", buf_size,
                                      "s:sync", shallow ? "top" : "full",
                                      "s:mode", reuse ? "existing" : "absolute-paths",
                                      "S:format", format,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d8c9308..cd331db 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -249,6 +249,8 @@ int qemuMonitorJSONDriveMirror(qemuMonitorPtr mon,
                                const char *file,
                                const char *format,
                                unsigned long long speed,
+                               unsigned int granularity,
+                               unsigned int buf_size,
                                unsigned int flags)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index e3fb4f7..afbf13a 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1176,7 +1176,7 @@ GEN_TEST_FUNC(qemuMonitorJSONRemoveNetdev, "net0")
 GEN_TEST_FUNC(qemuMonitorJSONDelDevice, "ide0")
 GEN_TEST_FUNC(qemuMonitorJSONAddDevice, "some_dummy_devicestr")
 GEN_TEST_FUNC(qemuMonitorJSONSetDrivePassphrase, "vda", "secret_passhprase")
-GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", NULL, 1024,
+GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", NULL, 1024, 0, 0,
               VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)
 GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", NULL, 1024)
 GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb", NULL, NULL)
-- 
1.9.3


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]