From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bi47u-0006il-Qu for qemu-devel@nongnu.org; Thu, 08 Sep 2016 14:32:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bi47q-0003dp-PE for qemu-devel@nongnu.org; Thu, 08 Sep 2016 14:32:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bi47q-0003db-Ha for qemu-devel@nongnu.org; Thu, 08 Sep 2016 14:32:38 -0400 References: <1473350399-29210-1-git-send-email-ashijeetacharya@gmail.com> From: Eric Blake Message-ID: Date: Thu, 8 Sep 2016 13:32:36 -0500 MIME-Version: 1.0 In-Reply-To: <1473350399-29210-1-git-send-email-ashijeetacharya@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9UjGdDDku56PewkQ8OR22m8iW20vC4n7c" Subject: Re: [Qemu-devel] [PATCH v4] Move max-bandwidth and downtime-limit into migrate_set_parameter for both hmp and qmp List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ashijeet Acharya , lcapitulino@redhat.com Cc: quintela@redhat.com, amit.shah@redhat.com, armbru@redhat.com, dgilbert@redhat.com, pbonzini@redhat.com, berrange@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9UjGdDDku56PewkQ8OR22m8iW20vC4n7c From: Eric Blake To: Ashijeet Acharya , lcapitulino@redhat.com Cc: quintela@redhat.com, amit.shah@redhat.com, armbru@redhat.com, dgilbert@redhat.com, pbonzini@redhat.com, berrange@redhat.com, qemu-devel@nongnu.org Message-ID: Subject: Re: [PATCH v4] Move max-bandwidth and downtime-limit into migrate_set_parameter for both hmp and qmp References: <1473350399-29210-1-git-send-email-ashijeetacharya@gmail.com> In-Reply-To: <1473350399-29210-1-git-send-email-ashijeetacharya@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/08/2016 10:59 AM, Ashijeet Acharya wrote: Long subject line; we strive to stay below 70 bytes so that you can prepend commit ids and still stay in an 80-column window. Also, you can look at 'git shortlog -30' to get a feel for average length. Better might be: migrate: move max-bandwidth and downtime-limit to migrate_set_parameter > Mark old-commands for speed and downtime as deprecated. > Move max-bandwidth and downtime-limit into migrate-set-parameters for > setting maximum migration speed and expected downtime limit parameters > respectively. > Change downtime units to milliseconds (only for new-command) and update= > the query part in both hmp and qmp qemu control interfaces. >=20 > Signed-off-by: Ashijeet Acharya > --- > hmp.c | 26 ++++++++++ Between the --- and diffstat is a nice place to tell us how v4 differs from v3, to focus the attention of reviewers that have already seen this once. > +++ b/hmp.c > @@ -304,6 +304,12 @@ void hmp_info_migrate_parameters(Monitor *mon, con= st QDict *qdict) > monitor_printf(mon, " %s: '%s'", > MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME= ], > params->tls_hostname ? : ""); > + monitor_printf(mon, " %s: %" PRId64, > + MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDT= H], > + params->max_bandwidth); Worth including a unit (bytes/second, right?) in the output here? > + monitor_printf(mon, " %s: %" PRId64, > + MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIM= IT], > + params->downtime_limit); and here (milliseconds, right)? Particularly since the two are on different scales. > monitor_printf(mon, "\n"); > } > =20 > @@ -1193,6 +1199,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDi= ct *qdict) > hmp_handle_error(mon, &err); > } > =20 > +/* Kept for old-commands compatibility */ > void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict) s/old-commands/backwards/ > { > double value =3D qdict_get_double(qdict, "value"); > @@ -1211,6 +1218,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, con= st QDict *qdict) > } > } > =20 > +/* Kept for old-commands compatibility */ > void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) and again > @@ -773,6 +779,10 @@ void qmp_migrate_set_parameters(bool has_compress_= level, > const char *tls_creds, > bool has_tls_hostname, > const char *tls_hostname, > + bool has_max_bandwidth, > + int64_t max_bandwidth, > + bool has_downtime_limit, > + int64_t downtime_limit, > Error **errp) > { > + if (has_downtime_limit) { > + downtime_limit *=3D 1000000; /* convert to nanoseconds */ > + Still risks overflow on user-provided input. You should validate that the user's input is reasonable, rather than accidentally setting a too-small limit when the user meant for something so long as to appear infinite. > +void qmp_migrate_set_speed(int64_t valuebw, Error **errp) > +{ > + bool has_compress_level =3D false; > + bool has_compress_threads =3D false; > + bool has_decompress_threads =3D false; > + bool has_cpu_throttle_initial =3D false; > + bool has_cpu_throttle_increment =3D false; > + bool has_tls_creds =3D false; > + bool has_tls_hostname =3D false; > + bool has_max_bandwidth =3D true; > + bool has_downtime_limit =3D false; > + const char *valuestr =3D NULL; > + long valueint =3D 0; > + Error *err =3D NULL; > + > + qmp_migrate_set_parameters(has_compress_level, valueint, > + has_compress_threads, valueint, > + has_decompress_threads, valueint, > + has_cpu_throttle_initial, valueint, > + has_cpu_throttle_increment, valueint, > + has_tls_creds, valuestr, > + has_tls_hostname, valuestr, > + has_max_bandwidth, valuebw, > + has_downtime_limit, valueint, > + &err); > + And you may want to wait for v5 until after I post my patch later today, so you can see what I meant in the v4 discussion about using the qapi features for shorter code. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --9UjGdDDku56PewkQ8OR22m8iW20vC4n7c Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJX0a7EAAoJEKeha0olJ0NqijUH/RxU9zr1G2ZXxGxwEjHg0zqx OVZ2ILjwdPXBkRME1jklGPkeyze3Drq3PGj+rewtNv8t1hzTsAK7hNnf7pPJd+pD rmda+3mJcFamumorHQEPsh7Z4hOw3u6D5hJ0SumDfPkzoDjcNv0rStpsDyKX0NWC ISruoN5iQ1aXIsE5YZIsh3AI32GJG72sw2/boYDwYLp7p5/i5ofcCzJ534lVJzxI 5hjU34yYlafO/HC45ISWDD4qCeDU8o7ANmTeKMInrlgAgXznoSGNrgE7XsPAgXg+ tujYFHtT9YHf3GWEUgJG98UA5bsxazevjAAH3JGrtk3Z9ssGIrZgk8ZJWGpTotk= =WPtB -----END PGP SIGNATURE----- --9UjGdDDku56PewkQ8OR22m8iW20vC4n7c--