diff mbox

[RFC,v1,4/5] VMState test: set the frequency of the vmstate testing process

Message ID 1404753484-26693-5-git-send-email-sanidhya.iiith@gmail.com
State New
Headers show

Commit Message

Sanidhya Kashyap July 7, 2014, 5:18 p.m. UTC
This patch introduces the mechanism to update the sleep interval - sinterval.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 hmp-commands.hx  | 15 +++++++++++++++
 hmp.c            | 14 ++++++++++++++
 hmp.h            |  1 +
 qapi-schema.json | 10 ++++++++++
 qmp-commands.hx  | 22 ++++++++++++++++++++++
 savevm.c         | 13 +++++++++++++
 6 files changed, 75 insertions(+)

Comments

Eric Blake July 7, 2014, 6:25 p.m. UTC | #1
On 07/07/2014 11:18 AM, Sanidhya Kashyap wrote:
> This patch introduces the mechanism to update the sleep interval - sinterval.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  hmp-commands.hx  | 15 +++++++++++++++
>  hmp.c            | 14 ++++++++++++++
>  hmp.h            |  1 +
>  qapi-schema.json | 10 ++++++++++
>  qmp-commands.hx  | 22 ++++++++++++++++++++++
>  savevm.c         | 13 +++++++++++++
>  6 files changed, 75 insertions(+)
> 

> +++ b/qapi-schema.json
> @@ -3492,3 +3492,13 @@
>  { 'command': 'test-vmstates',
>    'data': {'*times'     : 'int',
>             '*sinterval' : 'int' } }
> +
> +## @test-vmstates-set-sinterval
> +#
> +# sets the sleep interval between iterations of the vmstate testing process
> +# @sinterval: the updated sleep interval value

in what units?

> +#
> +# Since 2.1

2.2.

> +##
> +{ 'command' : 'test-vmstates-set-sinterval',
> +  'data'    : { 'sinterval': 'int' } }

This feels like a write-only command.  How do I query what it is
currently set to?

I'm also afraid that we aren't learning our lessons from migrate.
Adding one new command per new tunable does not scale very well in the
number of commands that need to be maintained or that have to be wired
up in management software; better is adding a single command pair for
set/get that can be easily introspected (to see when new tunables are
added), by taking an array of tunables where each tunable is a struct
containing name, type, and value information.  Something like:

{"execute":"test-vmstates-set-tuning", "arguments":{ "list":[
  { "name":"sinterval", "type":"int", "value":100 },
  { "name":"other", "type":"...", "value":... }
]}}
{"return": {}}

and a counterpart:

{"execute":"test-vmstates-get-tuning" }
{"return": [
  { "name":"sinterval", "type":"int", "value":100 },
  { "name":"other", "type":"...", "value":... }
]}


> +void qmp_test_vmstates_set_sinterval(int64_t sinterval, Error **errp)
> +{
> +    VMStateLogState *v = vmstate_current_state();
> +    if (sinterval < TEST_VMSTATE_MIN_INTERVAL_MS ||
> +        sinterval > TEST_VMSTATE_MAX_INTERVAL_MS) {
> +        error_setg(errp, "sleep interval value must be "
> +                   "in the defined range [%d, %d](ms)\n",
> +                   TEST_VMSTATE_MIN_INTERVAL_MS, TEST_VMSTATE_MAX_INTERVAL_MS);

This range limit should also be mentioned in the public API (.json) file.
Sanidhya Kashyap July 18, 2014, 6:59 p.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> This feels like a write-only command.  How do I query what it is 
> currently set to?
> 
> I'm also afraid that we aren't learning our lessons from migrate. 
> Adding one new command per new tunable does not scale very well in
> the number of commands that need to be maintained or that have to
> be wired up in management software; better is adding a single
> command pair for set/get that can be easily introspected (to see
> when new tunables are added), by taking an array of tunables where
> each tunable is a struct containing name, type, and value
> information.  Something like:
> 
> {"execute":"test-vmstates-set-tuning", "arguments":{ "list":[ {
> "name":"sinterval", "type":"int", "value":100 }, { "name":"other",
> "type":"...", "value":... } ]}} {"return": {}}
> 
> and a counterpart:
> 
> {"execute":"test-vmstates-get-tuning" } {"return": [ {
> "name":"sinterval", "type":"int", "value":100 }, { "name":"other",
> "type":"...", "value":... } ]}
> 
> 

Well, I have some issues about the implementation of the list which is
associated with the set-tuning one. Any patch illustrating the above
example which includes list will help me a lot.

Thanks.

- -- 
- -----

Sanidhya Kashyap
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTyW6KAAoJEFt9RLmoahlnFQkIAJL6z0jo72gEc67ISr6pNxyH
sxFbT67LNHhliWlVvohOPqC4e8hGOfXJb5gmPr2UFZUbVNKLrGc1Mh8t5OeRPR2y
3twA+3uUvpuJhc6F+KH4Z125Qirj7Uom9/PCtOYD9QMIAOEkD9SzD1S3ZpWL6I2r
+l0s2xkFScG9eHyqe6pJlKRIORm9FnKVhYS18bzXz1WWh1mJTjF6qXNfbCS3OyqT
9rk1ZcYyYei/JNoclgC7Mvxy5QpQxrhhjwDFK0G3vFaPte+K+CYpGHL8a5s0sPFb
OOZ29X+5Zz4wOx9btf0+LovqSwrLueHCdF/l65+jSNPRRsvB4MslQva/Rybs4Kg=
=DpKh
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index c492f3f..efe4354 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1803,6 +1803,21 @@  STEXI
 dumps and reads the device state's data from the memory for testing purpose
 ETEXI
 
+    {
+        .name       = "test-vmstates-set-sinterval",
+        .args_type  = "sinterval:i",
+        .params     = "sinterval",
+        .help       = "set the sleep interval for vmstates testing process\n\t\t\t"
+                      "sinterval: the new sleep interval value to replace the existing",
+        .mhandler.cmd = hmp_test_vmstates_set_sinterval,
+    },
+
+STEXI
+@item test_vmstates-set-sinterval @var{sinterval}
+@findex test-vmstates-set-sinterval
+Set the sinterval to @var{sinterval} (int) for vmstate testing process.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index 38ec5b3..7378727 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1728,3 +1728,17 @@  void hmp_test_vmstates(Monitor *mon, const QDict *qdict)
         error_free(err);
     }
 }
+
+void hmp_test_vmstates_set_sinterval(Monitor *mon, const QDict *qdict)
+{
+    int64_t sleep_interval = qdict_get_int(qdict, "sinterval");
+    Error *err = NULL;
+
+    qmp_test_vmstates_set_sinterval(sleep_interval, &err);
+
+    if (err) {
+        monitor_printf(mon, "test-vmstates-set-frequency: %s\n",
+                       error_get_pretty(err));
+        error_free(err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 9f00997..737dae2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -95,6 +95,7 @@  void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_info_memdev(Monitor *mon, const QDict *qdict);
 void hmp_test_vmstates(Monitor *mon, const QDict *qdict);
+void hmp_test_vmstates_set_sinterval(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 92acf91..2eb8bc4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3492,3 +3492,13 @@ 
 { 'command': 'test-vmstates',
   'data': {'*times'     : 'int',
            '*sinterval' : 'int' } }
+
+## @test-vmstates-set-sinterval
+#
+# sets the sleep interval between iterations of the vmstate testing process
+# @sinterval: the updated sleep interval value
+#
+# Since 2.1
+##
+{ 'command' : 'test-vmstates-set-sinterval',
+  'data'    : { 'sinterval': 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ccddabb..2c25dde 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3783,3 +3783,25 @@  Example:
         "sinterval": 100 } }
 <- { "return": {} }
 EQMP
+
+    {
+        .name       = "test-vmstates-set-sinterval",
+        .args_type  = "sinterval:i",
+        .mhandler.cmd_new = qmp_marshal_input_test_vmstates_set_sinterval,
+    },
+
+SQMP
+test-vmstates-set-sinterval
+--------------------
+
+Update the sleep interval for the remaining iterations
+
+Arguments:
+
+- "sinterval": the updated sleep interval between iterations (json-int)
+
+Example:
+
+-> { "execute": "test-vmstates-set-sinterval", "arguments": { "sinterval": 1024 } }
+<- { "return": {} }
+EQMP
diff --git a/savevm.c b/savevm.c
index 3713a56..8a81355 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1285,6 +1285,19 @@  void qmp_test_vmstates(bool has_times, int64_t times,
     timer_mod(v->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
 }
 
+void qmp_test_vmstates_set_sinterval(int64_t sinterval, Error **errp)
+{
+    VMStateLogState *v = vmstate_current_state();
+    if (sinterval < TEST_VMSTATE_MIN_INTERVAL_MS ||
+        sinterval > TEST_VMSTATE_MAX_INTERVAL_MS) {
+        error_setg(errp, "sleep interval value must be "
+                   "in the defined range [%d, %d](ms)\n",
+                   TEST_VMSTATE_MIN_INTERVAL_MS, TEST_VMSTATE_MAX_INTERVAL_MS);
+        return;
+    }
+    v->sleep_interval = sinterval;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
     QEMUFile *f;