[v3] Changing error message of QMP 'migrate_set_downtime' to seconds

Commit Message

Daniel Henrique Barboza Feb. 22, 2017, 3:17 p.m. UTC

Using QMP, the error message of 'migrate_set_downtime' was displaying
the values in milliseconds, being misleading with the command that
accepts the value in seconds:

{ "execute": "migrate_set_downtime", "arguments": {"value": 3000}}
{"error": {"class": "GenericError", "desc": "Parameter 'downtime_limit'
expects an integer in the range of 0 to 2000000 milliseconds"}}

This message is also seen in HMP when trying to set the same
parameter:

(qemu) migrate_set_parameter downtime-limit 3000000
Parameter 'downtime_limit' expects an integer in the range of 0 to
2000000 milliseconds

To allow for a proper error message when using QMP, a validation
of the user input was added in 'qmp_migrate_set_downtime'.

Signed-off-by: Daniel Henrique Barboza <[e-mail]>
---
 migration/migration.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Juan Quintela Feb. 22, 2017, 7:40 p.m. UTC | #1

Daniel Henrique Barboza <[e-mail]> wrote:
> Using QMP, the error message of 'migrate_set_downtime' was displaying
> the values in milliseconds, being misleading with the command that
> accepts the value in seconds:
>
> { "execute": "migrate_set_downtime", "arguments": {"value": 3000}}
> {"error": {"class": "GenericError", "desc": "Parameter 'downtime_limit'
> expects an integer in the range of 0 to 2000000 milliseconds"}}
>
> This message is also seen in HMP when trying to set the same
> parameter:
>
> (qemu) migrate_set_parameter downtime-limit 3000000
> Parameter 'downtime_limit' expects an integer in the range of 0 to
> 2000000 milliseconds
>
> To allow for a proper error message when using QMP, a validation
> of the user input was added in 'qmp_migrate_set_downtime'.
>
> Signed-off-by: Daniel Henrique Barboza <[e-mail]>

Reviewed-by: Juan Quintela <[e-mail]>

Queued.  Thanks.

Patch

diff --git a/migration/migration.c b/migration/migration.c
index c6ae69d..4fab538 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -49,6 +49,10 @@ 
  * for sending the last part */
 #define DEFAULT_MIGRATE_SET_DOWNTIME 300
 
+/* Maximum migrate downtime set to 2000 seconds */
+#define MAX_MIGRATE_DOWNTIME_SECONDS 2000
+#define MAX_MIGRATE_DOWNTIME (MAX_MIGRATE_DOWNTIME_SECONDS * 1000)
+
 /* Default compression thread count */
 #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8
 /* Default decompression thread count, usually decompression is at
@@ -843,10 +847,11 @@  void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
         return;
     }
     if (params->has_downtime_limit &&
-        (params->downtime_limit < 0 || params->downtime_limit > 2000000)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   "downtime_limit",
-                   "an integer in the range of 0 to 2000000 milliseconds");
+        (params->downtime_limit < 0 ||
+         params->downtime_limit > MAX_MIGRATE_DOWNTIME)) {
+        error_setg(errp, "Parameter 'downtime_limit' expects an integer in "
+                         "the range of 0 to %d milliseconds",
+                         MAX_MIGRATE_DOWNTIME);
         return;
     }
     if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) {
@@ -1289,6 +1294,13 @@  void qmp_migrate_set_speed(int64_t value, Error **errp)
 
 void qmp_migrate_set_downtime(double value, Error **errp)
 {
+    if (value < 0 || value > MAX_MIGRATE_DOWNTIME_SECONDS) {
+        error_setg(errp, "Parameter 'downtime_limit' expects an integer in "
+                         "the range of 0 to %d seconds",
+                         MAX_MIGRATE_DOWNTIME_SECONDS);
+        return;
+    }
+
     value *= 1000; /* Convert to milliseconds */
     value = MAX(0, MIN(INT64_MAX, value));