From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bi1QQ-0003LG-K4 for qemu-devel@nongnu.org; Thu, 08 Sep 2016 11:39:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bi1QN-0002uV-Pd for qemu-devel@nongnu.org; Thu, 08 Sep 2016 11:39:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33052) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bi1QN-0002uE-Hj for qemu-devel@nongnu.org; Thu, 08 Sep 2016 11:39:35 -0400 References: <1473169187-22750-1-git-send-email-ashijeetacharya@gmail.com> <01e7066e-b807-95a6-6a5e-3435651a949e@redhat.com> <87twdqzc6w.fsf@emacs.mitica> From: Eric Blake Message-ID: <132c6969-9cf8-4d5d-8bd3-93c914f26908@redhat.com> Date: Thu, 8 Sep 2016 10:39:33 -0500 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="PF48rwt04HqmMCKP6SkoTVOQmVCHDBCRl" Subject: Re: [Qemu-devel] [PATCH v3] 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 , quintela@redhat.com Cc: lcapitulino@redhat.com, amit.shah@redhat.com, armbru@redhat.com, "dgilbert@redhat.com" , Paolo Bonzini , "Daniel P. Berrange" , QEMU Developers This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --PF48rwt04HqmMCKP6SkoTVOQmVCHDBCRl From: Eric Blake To: Ashijeet Acharya , quintela@redhat.com Cc: lcapitulino@redhat.com, amit.shah@redhat.com, armbru@redhat.com, "dgilbert@redhat.com" , Paolo Bonzini , "Daniel P. Berrange" , QEMU Developers Message-ID: <132c6969-9cf8-4d5d-8bd3-93c914f26908@redhat.com> Subject: Re: [PATCH v3] Move max-bandwidth and downtime-limit into migrate_set_parameter for both hmp and qmp References: <1473169187-22750-1-git-send-email-ashijeetacharya@gmail.com> <01e7066e-b807-95a6-6a5e-3435651a949e@redhat.com> <87twdqzc6w.fsf@emacs.mitica> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/08/2016 10:30 AM, Ashijeet Acharya wrote: >>>> + if (has_downtime_limit) { >>>> + downtime_limit *=3D 1000000; /* convert to nanoseconds */ >>> >>> Are you sure this won't overflow? >> >> Ashijeet, Eric mere means that if downtime_limit is bigger that >> INT64_MAX/1000000, then you get an overflow with the multiplication. >=20 > Oh I get it now. >=20 >> Notice that if it overflows, the value is already quite big O:-) >> >> 2^63/1000/1000/1000/3600/24/365 >> 292.47 >> >> Allowing "only" 292 years of downtime should be enough for the time >> being (famous last words) O:-) >> >=20 > Haha! 292 years seems sufficient. :-) But it still means we should explicitly check that whatever number the user inputs is not wrapping around to a smaller actual downtime - especially since wraparound to a small number CAN be observed within a human lifespan, for some values of wraparound. Either give the user an error (their input was too big; preferred) or silently convert the user's too-large input to the maximum possible (they won't live long enough to notice the difference, not ideal, but if reporting an error is too hard, it is workable). >>>> + >>>> + qmp_migrate_set_parameters(has_compress_level, valueint, >>>> + has_compress_threads, valueint, >>> >>> Ugg. This looks gross. No need to name a bunch of variables set to >>> false, when you can instead use QAPI's boxed conventions to pass a >>> pointer to a struct, and rely on 0-initialization of the struct to le= ave >>> all the parameters that you don't care about unmentioned. >> >> We should change qmp_migrate_set_parameters to your new api. I fully >> agree that this is gross, but it is the way it was written, nothing to= >> do with this patch, really. >=20 > Yeah, you expressed your disgust about this in your 'to-do list of > migration mail' too I remember. Okay, I'll take the hint and submit a patch to convert migration to use the new boxed parameters from QAPI. It's an independent change, but you would then rebase your series on top of my patch (or if yours lands first, my work would remove the additions in yours, for some churn in the code base, but no real harm done). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --PF48rwt04HqmMCKP6SkoTVOQmVCHDBCRl 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/ iQEcBAEBCAAGBQJX0YY1AAoJEKeha0olJ0NqMKEH/iVJGcg0HtBZQeiNEAmc0Rcl G1ZQ9lYakHeioYCuWyj2C8EQyIl27Db5rFFbrRlO9iV/ThiKQMUHopEBoOkfqxpZ aRB6fLBSXXRlNjdfQfCJx9w4A8NmU5g3T/TJ3nAck8k5aRjrG8tbe7nD8O4/9ZIU PPQCdhPuxE5kRGUu2s0UUJVgmBO3XaIeMcHy1eycrwn1DEuI1hmWy2SjfIjvJ6ZA xCxdsQWwxibY16t0YmqyWAdZKgq6a0ELSUgRkWRMHJtVqHHViqar1zPYPNDemAnN kfPYgqcKvY8KT0BQ0YxDId07ur0mXnnD09BqTD1kmaMVh4SujrP8e6cigYTS544= =vRxV -----END PGP SIGNATURE----- --PF48rwt04HqmMCKP6SkoTVOQmVCHDBCRl--