1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v3 0/3] implement migrate-getmaxdowntime command
  4. View patch

[libvirt] [PATCH v3 2/3] qemu: Implement virDomainMigrateGetMaxDowntime

Scott Garfinkle posted 3 patches 8 years, 6 months ago
[libvirt] [PATCH v3 2/3] qemu: Implement virDomainMigrateGetMaxDowntime
Posted by Scott Garfinkle 8 years, 6 months ago 
From: Scott Garfinkle <seg@us.ibm.com>

---
 src/qemu/qemu_driver.c       | 51 ++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor.h      |  3 +++
 src/qemu/qemu_monitor_json.c |  4 ++++
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 16 +++++++++++++-
 src/remote_protocol-structs  |  8 +++++++
 6 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f7b4cc3..8b8ae57 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13147,6 +13147,56 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom,
     return ret;
 }
 
+
+static int
+qemuDomainMigrateGetMaxDowntime(virDomainPtr dom,
+                                unsigned long long *downtime,
+                                unsigned int flags)
+{
+    virQEMUDriverPtr driver = dom->conn->privateData;
+    virDomainObjPtr vm;
+    qemuDomainObjPrivatePtr priv;
+    qemuMonitorMigrationParams migparams;
+    int ret = -1;
+
+    virCheckFlags(0, -1);
+
+    if (!(vm = qemuDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainMigrateGetMaxDowntimeEnsureACL(dom->conn, vm->def) < 0)
+        goto cleanup;
+
+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+        goto cleanup;
+
+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        goto endjob;
+    }
+
+    priv = vm->privateData;
+    qemuDomainObjEnterMonitor(driver, vm);
+
+    if (!(ret = qemuMonitorGetMigrationParams(priv->mon, &migparams))) {
+        if (migparams.downtimeLimit_set)
+            *downtime = migparams.downtimeLimit;
+        else
+            ret = -1;
+    }
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        ret = -1;
+
+ endjob:
+    qemuDomainObjEndJob(driver, vm);
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+
 static int
 qemuDomainMigrateGetCompressionCache(virDomainPtr dom,
                                      unsigned long long *cacheSize,
@@ -20826,6 +20876,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
     .domainGetJobInfo = qemuDomainGetJobInfo, /* 0.7.7 */
     .domainGetJobStats = qemuDomainGetJobStats, /* 1.0.3 */
     .domainAbortJob = qemuDomainAbortJob, /* 0.7.7 */
+    .domainMigrateGetMaxDowntime = qemuDomainMigrateGetMaxDowntime, /* 3.6.0 */
     .domainMigrateSetMaxDowntime = qemuDomainMigrateSetMaxDowntime, /* 0.8.0 */
     .domainMigrateGetCompressionCache = qemuDomainMigrateGetCompressionCache, /* 1.0.3 */
     .domainMigrateSetCompressionCache = qemuDomainMigrateSetCompressionCache, /* 1.0.3 */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31f7e97..9805a33 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -627,6 +627,9 @@ struct _qemuMonitorMigrationParams {
      * whereas, some string value indicates we can support setting/clearing */
     char *migrateTLSAlias;
     char *migrateTLSHostname;
+
+    bool downtimeLimit_set;
+    unsigned long long downtimeLimit;
 };
 
 int qemuMonitorGetMigrationParams(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b8a6815..b7b809d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2703,6 +2703,10 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
     PARSE(cpuThrottleInitial, "cpu-throttle-initial");
     PARSE(cpuThrottleIncrement, "cpu-throttle-increment");
 
+    if (virJSONValueObjectGetNumberUlong(result, "downtime-limit",
+                                         &params->downtimeLimit) == 0)
+        params->downtimeLimit_set = true;
+
 #undef PARSE
 
     if ((tlsStr = virJSONValueObjectGetString(result, "tls-creds"))) {
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a57d25f..aa8d8a1 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8400,6 +8400,7 @@ static virHypervisorDriver hypervisor_driver = {
     .domainGetJobInfo = remoteDomainGetJobInfo, /* 0.7.7 */
     .domainGetJobStats = remoteDomainGetJobStats, /* 1.0.3 */
     .domainAbortJob = remoteDomainAbortJob, /* 0.7.7 */
+    .domainMigrateGetMaxDowntime = remoteDomainMigrateGetMaxDowntime, /* 3.6.0 */
     .domainMigrateSetMaxDowntime = remoteDomainMigrateSetMaxDowntime, /* 0.8.0 */
     .domainMigrateGetCompressionCache = remoteDomainMigrateGetCompressionCache, /* 1.0.3 */
     .domainMigrateSetCompressionCache = remoteDomainMigrateSetCompressionCache, /* 1.0.3 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index aa0aa38..e1f4e27 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2326,6 +2326,15 @@ struct remote_domain_abort_job_args {
 };
 
 
+struct remote_domain_migrate_get_max_downtime_args {
+    remote_nonnull_domain dom;
+    unsigned int flags;
+};
+
+struct remote_domain_migrate_get_max_downtime_ret {
+     unsigned hyper downtime; /* insert@1 */
+};
+
 struct remote_domain_migrate_set_max_downtime_args {
     remote_nonnull_domain dom;
     unsigned hyper downtime;
@@ -6064,7 +6073,12 @@ enum remote_procedure {
      * @generate: both
      * @acl: domain:write
      */
-    REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386
+    REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386,
 
+    /**
+     * @generate: both
+     * @acl: domain:migrate
+     */
+    REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_DOWNTIME = 387
 
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index a46fe37..5804e60 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -1778,6 +1778,13 @@ struct remote_domain_migrate_set_max_downtime_args {
         uint64_t                   downtime;
         u_int                      flags;
 };
+struct remote_domain_migrate_get_max_downtime_args {
+        remote_nonnull_domain      dom;
+        u_int                      flags;
+};
+struct remote_domain_migrate_get_max_downtime_ret {
+        uint64_t                   downtime;
+};
 struct remote_domain_migrate_get_compression_cache_args {
         remote_nonnull_domain      dom;
         u_int                      flags;
@@ -3233,4 +3240,5 @@ enum remote_procedure {
         REMOTE_PROC_DOMAIN_SET_VCPU = 384,
         REMOTE_PROC_DOMAIN_EVENT_BLOCK_THRESHOLD = 385,
         REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386,
+        REMOTE_PROC_DOMAIN_GET_MAX_DOWNTIME = 387
 };
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/3] qemu: Implement virDomainMigrateGetMaxDowntime
Posted by John Ferlan 8 years, 5 months ago 

On 07/28/2017 10:57 AM, Scott Garfinkle wrote:
> From: Scott Garfinkle <seg@us.ibm.com>
> 

Need a commit message. Typically this is something like "Set up wire and
protocol for xxx" (see commit id 20ffaf59d for the SetMaxDowntime example)

> ---
>  src/qemu/qemu_driver.c       | 51 ++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.h      |  3 +++
>  src/qemu/qemu_monitor_json.c |  4 ++++
>  src/remote/remote_driver.c   |  1 +
>  src/remote/remote_protocol.x | 16 +++++++++++++-
>  src/remote_protocol-structs  |  8 +++++++
>  6 files changed, 82 insertions(+), 1 deletion(-)
> 

This patch failed 'make check syntax-check' (see below)

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f7b4cc3..8b8ae57 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13147,6 +13147,56 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom,
>      return ret;
>  }
>  
> +
> +static int
> +qemuDomainMigrateGetMaxDowntime(virDomainPtr dom,
> +                                unsigned long long *downtime,
> +                                unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm;
> +    qemuDomainObjPrivatePtr priv;
> +    qemuMonitorMigrationParams migparams;

s/migparams;/migparams = { 0 };

just to be clear - so that downtimeLimit_set isn't 1 due to random stack
stuff

> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = qemuDomObjFromDomain(dom)))
> +        goto cleanup;

Could return -1 here

> +
> +    if (virDomainMigrateGetMaxDowntimeEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
> +        goto endjob;
> +    }
> +
> +    priv = vm->privateData;
> +    qemuDomainObjEnterMonitor(driver, vm);
> +
> +    if (!(ret = qemuMonitorGetMigrationParams(priv->mon, &migparams))) {

I go back and forth on this "if (!(ret = ())) vs. "if ((ret = ()) == 0)"
- I like the latter mainly because when I see (!( I'm thinking pointers.

> +        if (migparams.downtimeLimit_set)
> +            *downtime = migparams.downtimeLimit;
> +        else
> +            ret = -1;

I think this needs some sort of error message; otherwise, if the
"downtime-limit" doesn't exist from a "query-migrate-parameters" then
you'd get the fallback libvirt failed for some unknown reason error message.

Other places that return -1 here would all elicit some message...  Thus:

    if (qemuMonitorGetMigrationParams(priv->mon, &migparams) == 0) {
        if (migparams.downtimeLimit_set) {
            *downtime = migparams.downtimeLimit;
            ret = 0;
        } else {
            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                           _("Getting maximum downtime limit is not
supported"));
        }
    }

(or some error message such as that - I'm not sure if there is a way to
determine like the cache code does that the parameter exists.

> +    }
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;
> +
> + endjob:
> +    qemuDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    return ret;
> +}
> +

Extra space after too...

>  static int
>  qemuDomainMigrateGetCompressionCache(virDomainPtr dom,
>                                       unsigned long long *cacheSize,
> @@ -20826,6 +20876,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
>      .domainGetJobInfo = qemuDomainGetJobInfo, /* 0.7.7 */
>      .domainGetJobStats = qemuDomainGetJobStats, /* 1.0.3 */
>      .domainAbortJob = qemuDomainAbortJob, /* 0.7.7 */
> +    .domainMigrateGetMaxDowntime = qemuDomainMigrateGetMaxDowntime, /* 3.6.0 */

3.7.0

>      .domainMigrateSetMaxDowntime = qemuDomainMigrateSetMaxDowntime, /* 0.8.0 */
>      .domainMigrateGetCompressionCache = qemuDomainMigrateGetCompressionCache, /* 1.0.3 */
>      .domainMigrateSetCompressionCache = qemuDomainMigrateSetCompressionCache, /* 1.0.3 */
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 31f7e97..9805a33 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -627,6 +627,9 @@ struct _qemuMonitorMigrationParams {
>       * whereas, some string value indicates we can support setting/clearing */
>      char *migrateTLSAlias;
>      char *migrateTLSHostname;
> +
> +    bool downtimeLimit_set;
> +    unsigned long long downtimeLimit;
>  };
>  
>  int qemuMonitorGetMigrationParams(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index b8a6815..b7b809d 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2703,6 +2703,10 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
>      PARSE(cpuThrottleInitial, "cpu-throttle-initial");
>      PARSE(cpuThrottleIncrement, "cpu-throttle-increment");
>  
> +    if (virJSONValueObjectGetNumberUlong(result, "downtime-limit",
> +                                         &params->downtimeLimit) == 0)
> +        params->downtimeLimit_set = true;
> +
>  #undef PARSE

Put the new code after the #undef since it's not part of the PARSE

>  
>      if ((tlsStr = virJSONValueObjectGetString(result, "tls-creds"))) {
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index a57d25f..aa8d8a1 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -8400,6 +8400,7 @@ static virHypervisorDriver hypervisor_driver = {
>      .domainGetJobInfo = remoteDomainGetJobInfo, /* 0.7.7 */
>      .domainGetJobStats = remoteDomainGetJobStats, /* 1.0.3 */
>      .domainAbortJob = remoteDomainAbortJob, /* 0.7.7 */
> +    .domainMigrateGetMaxDowntime = remoteDomainMigrateGetMaxDowntime, /* 3.6.0 */

3.7.0

>      .domainMigrateSetMaxDowntime = remoteDomainMigrateSetMaxDowntime, /* 0.8.0 */
>      .domainMigrateGetCompressionCache = remoteDomainMigrateGetCompressionCache, /* 1.0.3 */
>      .domainMigrateSetCompressionCache = remoteDomainMigrateSetCompressionCache, /* 1.0.3 */
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index aa0aa38..e1f4e27 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -2326,6 +2326,15 @@ struct remote_domain_abort_job_args {
>  };
>  
>  
> +struct remote_domain_migrate_get_max_downtime_args {
> +    remote_nonnull_domain dom;
> +    unsigned int flags;
> +};
> +
> +struct remote_domain_migrate_get_max_downtime_ret {
> +     unsigned hyper downtime; /* insert@1 */
> +};
> +
>  struct remote_domain_migrate_set_max_downtime_args {
>      remote_nonnull_domain dom;
>      unsigned hyper downtime;
> @@ -6064,7 +6073,12 @@ enum remote_procedure {
>       * @generate: both
>       * @acl: domain:write
>       */
> -    REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386
> +    REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386,
>  
> +    /**
> +     * @generate: both
> +     * @acl: domain:migrate
> +     */
> +    REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_DOWNTIME = 387
>  
>  };
> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index a46fe37..5804e60 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -1778,6 +1778,13 @@ struct remote_domain_migrate_set_max_downtime_args {
>          uint64_t                   downtime;
>          u_int                      flags;
>  };
> +struct remote_domain_migrate_get_max_downtime_args {
> +        remote_nonnull_domain      dom;
> +        u_int                      flags;
> +};
> +struct remote_domain_migrate_get_max_downtime_ret {
> +        uint64_t                   downtime;
> +};

These two should be above the "set_max_downtime_args"
(check/syntax-check failure)


>  struct remote_domain_migrate_get_compression_cache_args {
>          remote_nonnull_domain      dom;
>          u_int                      flags;
> @@ -3233,4 +3240,5 @@ enum remote_procedure {
>          REMOTE_PROC_DOMAIN_SET_VCPU = 384,
>          REMOTE_PROC_DOMAIN_EVENT_BLOCK_THRESHOLD = 385,
>          REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386,
> +        REMOTE_PROC_DOMAIN_GET_MAX_DOWNTIME = 387

This needs to be:

REMOTE_PROC_DOMAIN_MIGRATE_GET_MAX_DOWNTIME = 387,

from check/syntax-check failure


John

>  };
> 

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

Patchew 2.1-devel-8c64711

© 2016 - 2026 Red Hat, Inc.