From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44576) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhBYw-0001xM-BG for qemu-devel@nongnu.org; Tue, 06 Sep 2016 04:16:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhBYr-0007gV-AN for qemu-devel@nongnu.org; Tue, 06 Sep 2016 04:16:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47082) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhBYr-0007gR-4m for qemu-devel@nongnu.org; Tue, 06 Sep 2016 04:16:53 -0400 From: Markus Armbruster References: <20160905111620.GA22359@work-vm> <1473075436-32489-1-git-send-email-ashijeetacharya@gmail.com> <122bc56c-0025-695a-8e2f-63aa03377879@redhat.com> <20160905155602.GK24656@redhat.com> Date: Tue, 06 Sep 2016 10:16:50 +0200 In-Reply-To: <20160905155602.GK24656@redhat.com> (Daniel P. Berrange's message of "Mon, 5 Sep 2016 16:56:02 +0100") Message-ID: <87eg4xbfyl.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2] Move migrate_set_speed and migrate_set_downtime into migrate_set_parameter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: Paolo Bonzini , quintela@redhat.com, QEMU Developers , "dgilbert@redhat.com" , Ashijeet Acharya , amit.shah@redhat.com, lcapitulino@redhat.com "Daniel P. Berrange" writes: > On Mon, Sep 05, 2016 at 03:16:04PM +0200, Paolo Bonzini wrote: >> >> >> On 05/09/2016 15:09, Ashijeet Acharya wrote: >> >>> >> + "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?,migrate-set-speed:o?,migrate-set-downtime:T?", >> >> > >> >> > Same here about the names. Also use "i" for QMP commands. >> > I think we will have to use "T" for downtime at-least otherwise you >> > cant use float values for setting seconds like "0.2" for example. >> > No issues using "i" with bandwidth though. >> >> Right, I should have mentioned that my remark was about the "o" (it >> caught my eye because of your change to hmp-commands.hx). Sorry! > > I've always thought it a rather wierd to use fractional seconds for > downtime. It sort of made sense for HMP, but should really never have > been carried over into QMP. IMHO, we should make it just take integer > milliseconds for downtime with the new API. Seconds are a fine unit, and using floating-point numbers isn't weird, especially not in JSON. What's actually weird is our zoo of time units: seconds (migrate_set_downtime), milliseconds (MigrationInfo), microseconds (NetdevTapOptions), nanoseconds (BlockDeviceTimedStats), seconds + fractional nanoseconds (SnapshotInfo), ... No adult supervision. We should've picked *one* time unit for QMP. The obvious time unit is seconds. Nanoseconds would've worked, too. Time units are probably beyond repair now. We can still try to make them at least locally consistent, i.e. stick to what related interfaces use.