From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57927) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEgKr-0006nD-Fm for qemu-devel@nongnu.org; Fri, 23 Jan 2015 10:39:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YEgKo-0007RM-9R for qemu-devel@nongnu.org; Fri, 23 Jan 2015 10:39:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44438) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEgKo-0007RA-1n for qemu-devel@nongnu.org; Fri, 23 Jan 2015 10:39:46 -0500 Message-ID: <54C26B3D.2040909@redhat.com> Date: Fri, 23 Jan 2015 08:39:41 -0700 From: Eric Blake MIME-Version: 1.0 References: <1418347746-15829-1-git-send-email-liang.z.li@intel.com> <1418347746-15829-13-git-send-email-liang.z.li@intel.com> In-Reply-To: <1418347746-15829-13-git-send-email-liang.z.li@intel.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mwDbfQjjrcWUtH7O8N0K95sEsDDJ7Pgdi" Subject: Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liang Li , qemu-devel@nongnu.org Cc: yang.z.zhang@intel.com, lcapitulino@redhat.com, armbru@redhat.com, dgilbert@redhat.com, quintela@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --mwDbfQjjrcWUtH7O8N0K95sEsDDJ7Pgdi Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/11/2014 06:29 PM, Liang Li wrote: > Add the qmp and hmp commands to tune the parameters used in live > migration. >=20 > Signed-off-by: Liang Li > Signed-off-by: Yang Zhang > --- > hmp-commands.hx | 15 ++++++++++ > hmp.c | 32 +++++++++++++++++++++ > hmp.h | 3 ++ > include/migration/migration.h | 4 +-- > migration.c | 66 +++++++++++++++++++++++++++++++++++= -------- > monitor.c | 18 ++++++++++++ > qapi-schema.json | 44 +++++++++++++++++++++++++++++ > qmp-commands.hx | 23 +++++++++++++++ > 8 files changed, 190 insertions(+), 15 deletions(-) >=20 > +++ b/hmp.h > @@ -63,6 +63,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qd= ict); > void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict); > void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict); > void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict); > +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict); > void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict); > void hmp_set_password(Monitor *mon, const QDict *qdict); > void hmp_expire_password(Monitor *mon, const QDict *qdict); > @@ -111,6 +112,8 @@ void watchdog_action_completion(ReadLineState *rs, = int nb_args, > const char *str); > void migrate_set_capability_completion(ReadLineState *rs, int nb_args,= > const char *str); > +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args, > + const char *str); Off-by-one on indentation. > @@ -292,6 +295,41 @@ void qmp_migrate_set_capabilities(MigrationCapabil= ityStatusList *params, > } > } > =20 > +void qmp_migrate_set_parameters(MigrationParameterStatusList *params, > + Error **errp) Indentation is off. > +{ > + MigrationState *s =3D migrate_get_current(); > + MigrationParameterStatusList *p; > + > + for (p =3D params; p; p =3D p->next) { > + switch (p->value->parameter) { > + case MIGRATION_PARAMETER_COMPRESS_LEVEL: > + if (p->value->value < 0 || p->value->value > 9) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "compres= s_level", > + "is invalied, it should be in the rang of 0 to 9")= ; s/invalied/invalid/; s/rang/range/ > + if (p->value->value < 1 || p->value->value > 255) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, > + "(de)compress_threads", > + "is invalied, it should be in the rang of 1 to 255= "); and again > +++ b/monitor.c > @@ -4544,6 +4544,24 @@ void migrate_set_capability_completion(ReadLineS= tate *rs, int nb_args, > } > } > =20 > +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args, > + const char *str) Indentation is off. > +++ b/qapi-schema.json > @@ -540,6 +540,50 @@ > ## > { 'command': 'query-migrate-capabilities', 'returns': ['MigrationCap= abilityStatus']} > =20 > +# @MigrationParameter > +# > +# Migration parameters enumeration > +# > +# @compress-level:Set the compression level to be used in live migrati= on, s/:/: / > +# the compression level is an integer between 0 and 9, where = 0 means > +# no compression, 1 means the best compression speed, and 9 m= eans best > +# compression ratio which will consume more CPU. > +# > +# @compress-threads: Set compression thread count to be used in live m= igration, > +# the compression thread count is an integer between 1 and 25= 5. > +# > +# @decompress-threads: Set decompression thread count to be used in li= ve migration, > +# the decompression thread count is an integer between 1 and = 255. > +# > +# Since: 2.3 > +## > +{ 'enum': 'MigrationParameter', > + 'data': ['compress-level', 'compress-threads', 'decompress-threads']= } > +## > +# @MigrationParameterStatus > +# > +# Migration parameter information > +# > +# @parameter: parameter enum > +# > +# @value: parameter value int > +# > +# Since: 2.3 > +## > +{ 'type': 'MigrationParameterStatus', > + 'data': { 'parameter' : 'MigrationParameter', 'value' : 'int' } } Doesn't allow for non-integer parameters. Not necessarily fatal for input only (I think a flat union could be utilized with a MigrationParameter as the discriminator to allow non-int values later on)= =2E.. > +## > +# @migrate-set-parameters > +# > +# Set the following migration parameters (like compress-level) > +# > +# @parameters: json array of parameter modifications to make > +# > +# Since: 2.3 > +## > +{ 'command': 'migrate-set-parameters', > + 'data': { 'parameters': ['MigrationParameterStatus'] } } =2E..but on output, we might confuse callers by outputting non-int values= unless we plan _from the start_ to support them. That is, I think we wan= t: { 'type': 'MigrationParameterBase', 'data': { 'parameter': 'MigrationParameter' } } { 'type': 'MigrationParameterInt', 'data': { 'value': 'int' } } { 'union': 'MigrationParameterStatus', 'base': 'MigrationParameterBase', 'discriminator': 'parameter', 'data': { 'compress-level': 'MigrationParameterInt', 'compress-threads': 'MigrationParameterInt', 'decompress-threads': 'MigrationParameterInt' } } to make it obvious to callers that they must be prepared to accept non-int values down the road if we ever extend the union to add another parameter with a different type. > SQMP > +migrate-set-parameters > +---------------------- > + > +Set migration parameters > + > +- "compress-leve": multiple compression thread support s/leve/level/ Missing two of the three defined parameters. I hate write-only interfaces. I'm hoping you add the query of parameters in a later patch - but I'd prefer you squash it into one patch= =2E --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --mwDbfQjjrcWUtH7O8N0K95sEsDDJ7Pgdi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJUwms9AAoJEKeha0olJ0NqDywH/3AGSmyzxuwR+V/Rjy/zbh6n WJ4JcUyqC9aOZCgu/B4yl5xuFeetulfBGXdGV1fpmcbBALuFsD88z/OyZgIHc9/h IzZhb5t9FJlksRunSHh1t97xFbFTdNxwKvZcKzzYFtPIQbdCZHacamVtrovamVtN uB7ICLrEdJ0M1o8VzYdFdF2YPba4WJM/FLkDqBWIabxz9Ykahr70SlJGMRfuRmKd jp+4UY+mfUxRV/ggHh617bHEwzd6sNJ4Uj3Qzh14HJpmO/ojCYko+L5xmnLh07+9 gO+6Qs987saPKK7EClD81yGyCmNXrpyJRK6QTVIByYrmg6SYkg38Zpg9wYxlaxs= =H2l8 -----END PGP SIGNATURE----- --mwDbfQjjrcWUtH7O8N0K95sEsDDJ7Pgdi--