Re: [Qemu-devel] [PATCH RFC v2 09/12] VMState test: update period of vmstate testing process

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2482 bytes --]

On 07/25/2014 09:39 AM, Sanidhya Kashyap wrote:
> No particular change, except variable name. Since I am not modifying other
> variables, so I have not made the command generic.

This sentence feels like changelog information compared to v1; as such,
it belongs...

> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---

...here, for use in the mail but not part of the commit itself.  On the
other hand, the commit itself should mention the name of the QMP command
it is adding (I shouldn't have to read the body of the patch to see what
the command name is).

>  qapi-schema.json | 12 ++++++++++++
>  qmp-commands.hx  | 23 +++++++++++++++++++++++
>  savevm.c         | 13 +++++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a12872f..13e922e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3562,3 +3562,15 @@
>  # Since 2.2
>  ##
>  { 'command': 'query-test-vmstates', 'returns': 'VMStateLogStateInfo' }
> +
> +##
> +# @test-vmstates-set-period
> +#
> +# sets the sleep interval between iterations of the vmstate testing process
> +#
> +# @period: the updated sleep interval value (in milliseconds)
> +#
> +# Since 2.2
> +##
> +{ 'command' : 'test-vmstates-set-period',
> +  'data'    : { 'period': 'int' } }

Is it possible that we might add other tunables in the future?  If so,
this command is not very scalable (we would be adding one command per
new tunable).  On the other hand, adding complexity to achieve a generic
setter is not worth it if we only expect one tunable.

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

This looks like it takes effect whether or not a vmstate test is
underway.  Does this impact the default of the next vmstate test to
start, in the case where that command doesn't supply a value for period
but relies on the default?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]