From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53101 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P5F9c-0000BC-Hs for qemu-devel@nongnu.org; Mon, 11 Oct 2010 05:58:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P5F9b-0007rz-J3 for qemu-devel@nongnu.org; Mon, 11 Oct 2010 05:58:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43741) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P5F9b-0007re-C8 for qemu-devel@nongnu.org; Mon, 11 Oct 2010 05:58:47 -0400 Message-ID: <4CB2DFD3.5010201@redhat.com> Date: Mon, 11 Oct 2010 11:58:43 +0200 From: Jes Sorensen MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 5/7] Switch migrate_set_speed() to take an 'o' argument rather than a float. References: <1286529360-5715-1-git-send-email-Jes.Sorensen@redhat.com> <1286529360-5715-6-git-send-email-Jes.Sorensen@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: pbonzini@redhat.com, Anthony Liguori , qemu-devel@nongnu.org On 10/11/10 11:03, Markus Armbruster wrote: > > As noted before, this is an incompatible change of the human monitor > command: unit now defaults to 'M'. This must be noted *prominently* in > the commit message. Best in the subject. > > Incompatible changes can break tools. Quick grep of libvirt: > > src/qemu/qemu_monitor_json.c: cmd = qemuMonitorJSONMakeCommand("migrate_set_speed", > src/qemu/qemu_monitor_text.c: if (virAsprintf(&cmd, "migrate_set_speed %lum", bandwidth) < 0) { > > Looks like we're safe here. > > Anthony, is this incompatibility okay? I agree it is incompatible, however it was a conscious decision to get rid of the inconsistency we had around various places for these types of arguments. If there is strong opposition to this change, then I can mangle the interface to allow for the old, but IMHO bad, default of the monitor. > Help should be updated in the same patch; please squash 6/7 into 5/7. Ok will do. Cheers, Jes