| Message ID | 1477979405-16264-1-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/01/2016 12:50 AM, zhanghailiang wrote: > We forgot to assign true to params->has_x_checkpoint_delay parameter > in qmp_query_migrate_parameters. > > Without this, qmp command 'query-migrate-parameters' doesn't show the > default value for x-checkpoint-delay option. > It doesn't influence output of hmp command 'info migrate_parameters'. Well, only because the current code doesn't forcefully assign missing optional parameters to any other value. But HMP was relying on unspecified behavior, that could have broken with any other qapi change. I might word the commit message: This also fixes the fact that HMP was relying on unspecified behavior by reading x_checkpoint_delay without checking has_x_checkpoint_delay. Up to the maintainer, though, since the patch itself is fine. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > hmp.c | 1 + > migration/migration.c | 1 + > 2 files changed, 2 insertions(+) Reviewed-by: Eric Blake <eblake@redhat.com>
On 2016/11/1 22:27, Eric Blake wrote: > On 11/01/2016 12:50 AM, zhanghailiang wrote: >> We forgot to assign true to params->has_x_checkpoint_delay parameter >> in qmp_query_migrate_parameters. >> >> Without this, qmp command 'query-migrate-parameters' doesn't show the >> default value for x-checkpoint-delay option. >> It doesn't influence output of hmp command 'info migrate_parameters'. > > Well, only because the current code doesn't forcefully assign missing > optional parameters to any other value. But HMP was relying on > unspecified behavior, that could have broken with any other qapi change. > > I might word the commit message: > > This also fixes the fact that HMP was relying on unspecified behavior by > reading x_checkpoint_delay without checking has_x_checkpoint_delay. > > Up to the maintainer, though, since the patch itself is fine. > OK, thanks, I'd like to fix the message as your said :) >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> hmp.c | 1 + >> migration/migration.c | 1 + >> 2 files changed, 2 insertions(+) > > Reviewed-by: Eric Blake <eblake@redhat.com> >
diff --git a/hmp.c b/hmp.c index b5e3f54..02103df 100644 --- a/hmp.c +++ b/hmp.c @@ -318,6 +318,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, " %s: %" PRId64 " milliseconds", MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT], params->downtime_limit); + assert(params->has_x_checkpoint_delay); monitor_printf(mon, " %s: %" PRId64, MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY], params->x_checkpoint_delay); diff --git a/migration/migration.c b/migration/migration.c index e331f28..f498ab8 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -593,6 +593,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->max_bandwidth = s->parameters.max_bandwidth; params->has_downtime_limit = true; params->downtime_limit = s->parameters.downtime_limit; + params->has_x_checkpoint_delay = true; params->x_checkpoint_delay = s->parameters.x_checkpoint_delay; return params;
We forgot to assign true to params->has_x_checkpoint_delay parameter in qmp_query_migrate_parameters. Without this, qmp command 'query-migrate-parameters' doesn't show the default value for x-checkpoint-delay option. It doesn't influence output of hmp command 'info migrate_parameters'. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- hmp.c | 1 + migration/migration.c | 1 + 2 files changed, 2 insertions(+)