Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds
Eric Blake writes: > On 02/17/2017 01:01 PM, Daniel Henrique Barboza wrote: > 200)) { +(params->downtime_limit < 0 || + params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "downtime_limit", - "an integer in the range of 0 to 200 milliseconds"); + "an integer in the range of 0 to 2000 seconds"); >>> Perhaps you could use %d and set MAX_MIGRATE_SET_DOWNTIME to 2000? >>> Though perhaps the migration maintainers are okay with the patch as is. >> >> I did that at first but I got errors on "error_setg" about the extra >> parameter. > > Ah, right, because QERR_INVALID_PARAMETER_VALUE is a macro that expands > to a fixed printf-style format string where you have to know how many > exact arguments it further expects. The only way around that is to > open-code the error message you want, instead of forcing the use of the > awkward macro. Go ahead and open-code whenever that results in better error messages.
Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds
On 02/17/2017 01:01 PM, Daniel Henrique Barboza wrote: >>> 200)) { >>> +(params->downtime_limit < 0 || >>> + params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) { >>> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, >>> "downtime_limit", >>> - "an integer in the range of 0 to 200 >>> milliseconds"); >>> + "an integer in the range of 0 to 2000 seconds"); >> Perhaps you could use %d and set MAX_MIGRATE_SET_DOWNTIME to 2000? >> Though perhaps the migration maintainers are okay with the patch as is. > > I did that at first but I got errors on "error_setg" about the extra > parameter. Ah, right, because QERR_INVALID_PARAMETER_VALUE is a macro that expands to a fixed printf-style format string where you have to know how many exact arguments it further expects. The only way around that is to open-code the error message you want, instead of forcing the use of the awkward macro. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds
On 02/17/2017 03:37 PM, Paolo Bonzini wrote: On 17/02/2017 18:26, Daniel Henrique Barboza wrote: The previous error message was displaying the values in miliseconds, 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 200 milliseconds"}} This patch changes it to '2000 seconds' to keep consistency with the expected parameter. Signed-off-by: Daniel Henrique Barboza --- migration/migration.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index c6ae69d..2dc63b1 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -49,6 +49,9 @@ * for sending the last part */ #define DEFAULT_MIGRATE_SET_DOWNTIME 300 +/* Maximum migrate downtime set to 2000*1000 miliseconds */ +#define MAX_MIGRATE_SET_DOWNTIME (2000 * 1000) + /* Default compression thread count */ #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8 /* Default decompression thread count, usually decompression is at @@ -843,10 +846,11 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp) return; } if (params->has_downtime_limit && -(params->downtime_limit < 0 || params->downtime_limit > 200)) { +(params->downtime_limit < 0 || + params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "downtime_limit", - "an integer in the range of 0 to 200 milliseconds"); + "an integer in the range of 0 to 2000 seconds"); Perhaps you could use %d and set MAX_MIGRATE_SET_DOWNTIME to 2000? Though perhaps the migration maintainers are okay with the patch as is. I did that at first but I got errors on "error_setg" about the extra parameter. I even considered using sprintf to format the string but I was afraid it would be a little overkill. Daniel Paolo return; } if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) {
Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds
On 17/02/2017 18:26, Daniel Henrique Barboza wrote: > The previous error message was displaying the values in miliseconds, > 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 200 milliseconds"}} > > This patch changes it to '2000 seconds' to keep consistency with > the expected parameter. > > Signed-off-by: Daniel Henrique Barboza > --- > migration/migration.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index c6ae69d..2dc63b1 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -49,6 +49,9 @@ > * for sending the last part */ > #define DEFAULT_MIGRATE_SET_DOWNTIME 300 > > +/* Maximum migrate downtime set to 2000*1000 miliseconds */ > +#define MAX_MIGRATE_SET_DOWNTIME (2000 * 1000) > + > /* Default compression thread count */ > #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8 > /* Default decompression thread count, usually decompression is at > @@ -843,10 +846,11 @@ void qmp_migrate_set_parameters(MigrationParameters > *params, Error **errp) > return; > } > if (params->has_downtime_limit && > -(params->downtime_limit < 0 || params->downtime_limit > 200)) { > +(params->downtime_limit < 0 || > + params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) { > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, > "downtime_limit", > - "an integer in the range of 0 to 200 milliseconds"); > + "an integer in the range of 0 to 2000 seconds"); Perhaps you could use %d and set MAX_MIGRATE_SET_DOWNTIME to 2000? Though perhaps the migration maintainers are okay with the patch as is. Paolo > return; > } > if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) { >