[PULL,24/28] migration: Make events a capability

Make check fails with events.  THis is due to the parser/lexer that it
uses.  Just in case that they are more broken parsers, just only send
events when there are capabilities.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/migration.h |  1 +
 migration/migration.c         | 20 ++++++++++++++++++--
 qapi-schema.json              |  5 ++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

At 2015/7/7 21:09, Juan Quintela Wrote:
> Make check fails with events.  THis is due to the parser/lexer that it
> uses.  Just in case that they are more broken parsers, just only send
> events when there are capabilities.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   include/migration/migration.h |  1 +
>   migration/migration.c         | 20 ++++++++++++++++++--
>   qapi-schema.json              |  5 ++++-
>   3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index a308ecc..b2711ef 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -177,6 +177,7 @@ bool migrate_use_compression(void);
>   int migrate_compress_level(void);
>   int migrate_compress_threads(void);
>   int migrate_decompress_threads(void);
> +bool migrate_use_events(void);
> 
>   void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
>   void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
> diff --git a/migration/migration.c b/migration/migration.c
> index d8415c4..cd32eac 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -205,6 +205,14 @@ void register_global_state(void)
>       vmstate_register(NULL, 0, &vmstate_globalstate, &global_state);
>   }
> 
> +static void migrate_generate_event(int new_state)
> +{
> +    if (migrate_use_events()) {
> +        qapi_event_send_migration(new_state, &error_abort);
> +        trace_migrate_set_state(new_state);
> +    }
> +}
> +
>   /*
>    * Called on -incoming with a defer: uri.
>    * The migration can be started later after any parameters have been
> @@ -511,8 +519,7 @@ void qmp_migrate_set_parameters(bool has_compress_level,
>   static void migrate_set_state(MigrationState *s, int old_state, int new_state)
>   {
>       if (atomic_cmpxchg(&s->state, old_state, new_state) == old_state) {
> -        qapi_event_send_migration(new_state, &error_abort);
> -        trace_migrate_set_state(new_state);
> +        migrate_generate_event(new_state);

Why moving trace_* to migrate_generate_event()? trace event should be
controlled
by the monitor command trace-event xxx on/off. It is very strange if the
user turns
the trace event on, but gets no output.

Thanks
Wen Congyang

>       }
>   }
> 
> @@ -862,6 +869,15 @@ int migrate_decompress_threads(void)
>       return s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
>   }
> 
> +bool migrate_use_events(void)
> +{
> +    MigrationState *s;
> +
> +    s = migrate_get_current();
> +
> +    return s->enabled_capabilities[MIGRATION_CAPABILITY_EVENTS];
> +}
> +
>   int migrate_use_xbzrle(void)
>   {
>       MigrationState *s;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 106008c..1285b8c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -523,6 +523,9 @@
>   #          minimize migration traffic. The feature is disabled by default.
>   #          (since 2.4 )
>   #
> +# @events: generate events for each migration state change
> +#          (since 2.4 )
> +#
>   # @auto-converge: If enabled, QEMU will automatically throttle down the guest
>   #          to speed up convergence of RAM migration. (since 1.6)
>   #
> @@ -530,7 +533,7 @@
>   ##
>   { 'enum': 'MigrationCapability',
>     'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
> -           'compress'] }
> +           'compress', 'events'] }
> 
>   ##
>   # @MigrationCapabilityStatus
>

Wen Congyang <ghostwcy@gmail.com> wrote:
> At 2015/7/7 21:09, Juan Quintela Wrote:
>> Make check fails with events.  THis is due to the parser/lexer that it
>> uses.  Just in case that they are more broken parsers, just only send
>> events when there are capabilities.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>   include/migration/migration.h |  1 +
>>   migration/migration.c         | 20 ++++++++++++++++++--
>>   qapi-schema.json              |  5 ++++-
>>   3 files changed, 23 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>> index a308ecc..b2711ef 100644
>> --- a/include/migration/migration.h
>> +++ b/include/migration/migration.h
>> @@ -177,6 +177,7 @@ bool migrate_use_compression(void);
>>   int migrate_compress_level(void);
>>   int migrate_compress_threads(void);
>>   int migrate_decompress_threads(void);
>> +bool migrate_use_events(void);
>> 
>>   void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
>>   void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
>> diff --git a/migration/migration.c b/migration/migration.c
>> index d8415c4..cd32eac 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -205,6 +205,14 @@ void register_global_state(void)
>>       vmstate_register(NULL, 0, &vmstate_globalstate, &global_state);
>>   }
>> 
>> +static void migrate_generate_event(int new_state)
>> +{
>> +    if (migrate_use_events()) {
>> +        qapi_event_send_migration(new_state, &error_abort);
>> +        trace_migrate_set_state(new_state);
>> +    }
>> +}
>> +
>>   /*
>>    * Called on -incoming with a defer: uri.
>>    * The migration can be started later after any parameters have been
>> @@ -511,8 +519,7 @@ void qmp_migrate_set_parameters(bool has_compress_level,
>>   static void migrate_set_state(MigrationState *s, int old_state, int new_state)
>>   {
>>       if (atomic_cmpxchg(&s->state, old_state, new_state) == old_state) {
>> -        qapi_event_send_migration(new_state, &error_abort);
>> -        trace_migrate_set_state(new_state);
>> +        migrate_generate_event(new_state);
>
> Why moving trace_* to migrate_generate_event()? trace event should be
> controlled
> by the monitor command trace-event xxx on/off. It is very strange if the
> user turns
> the trace event on, but gets no output.

Ok, will send another patch with one line change.

I put the set_state event as migrate_event one, but can accept the other
meanining.

Thanks, Juan.

>
> Thanks
> Wen Congyang
>
>>       }
>>   }
>> 
>> @@ -862,6 +869,15 @@ int migrate_decompress_threads(void)
>>       return s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
>>   }
>> 
>> +bool migrate_use_events(void)
>> +{
>> +    MigrationState *s;
>> +
>> +    s = migrate_get_current();
>> +
>> +    return s->enabled_capabilities[MIGRATION_CAPABILITY_EVENTS];
>> +}
>> +
>>   int migrate_use_xbzrle(void)
>>   {
>>       MigrationState *s;
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 106008c..1285b8c 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -523,6 +523,9 @@
>>   #          minimize migration traffic. The feature is disabled by default.
>>   #          (since 2.4 )
>>   #
>> +# @events: generate events for each migration state change
>> +#          (since 2.4 )
>> +#
>>   # @auto-converge: If enabled, QEMU will automatically throttle down the guest
>>   #          to speed up convergence of RAM migration. (since 1.6)
>>   #
>> @@ -530,7 +533,7 @@
>>   ##
>>   { 'enum': 'MigrationCapability',
>>     'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
>> -           'compress'] }
>> +           'compress', 'events'] }
>> 
>>   ##
>>   # @MigrationCapabilityStatus
>>

On Tue, Jul 07, 2015 at 15:09:05 +0200, Juan Quintela wrote:
> Make check fails with events.  THis is due to the parser/lexer that it
> uses.  Just in case that they are more broken parsers, just only send
> events when there are capabilities.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
...
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 106008c..1285b8c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -523,6 +523,9 @@
>  #          minimize migration traffic. The feature is disabled by default.
>  #          (since 2.4 )
>  #
> +# @events: generate events for each migration state change
> +#          (since 2.4 )
> +#
>  # @auto-converge: If enabled, QEMU will automatically throttle down the guest
>  #          to speed up convergence of RAM migration. (since 1.6)
>  #
> @@ -530,7 +533,7 @@
>  ##
>  { 'enum': 'MigrationCapability',
>    'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
> -           'compress'] }
> +           'compress', 'events'] }
> 

Perhaps I messed something up, but I don't see this new capability
documented in qmp-commands.hx and
{"execute":"query-migrate-capabilities"} does not report its status:
    {
      "return": [
        {
          "state": false,
          "capability": "xbzrle"
        },
        {
          "state": false,
          "capability": "rdma-pin-all"
        },
        {
          "state": false,
          "capability": "auto-converge"
        },
        {
          "state": false,
          "capability": "zero-blocks"
        },
        {
          "state": false,
          "capability": "compress"
        }
      ]
    }

Blindly setting it does not work either:
    {
      "execute": "migrate-set-capabilities",
      "arguments": {
        "capabilities": [
          {
            "capability": "events",
            "state": "true"
          }
        ]
      }
    }

returns
    {
      "error": {
        "class": "GenericError",
        "desc": "Invalid parameter 'events'"
      }
    }

Jirka