From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34254) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUJxH-0005W9-6t for qemu-devel@nongnu.org; Mon, 22 Apr 2013 12:51:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUJxC-0001KF-FF for qemu-devel@nongnu.org; Mon, 22 Apr 2013 12:51:03 -0400 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:39021) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUJxB-0001BY-VG for qemu-devel@nongnu.org; Mon, 22 Apr 2013 12:50:58 -0400 Received: from /spool/local by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 23 Apr 2013 02:40:51 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id CE0E52BB0051 for ; Tue, 23 Apr 2013 02:50:11 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3MGo4Xv11993504 for ; Tue, 23 Apr 2013 02:50:05 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r3MGo9YD029248 for ; Tue, 23 Apr 2013 02:50:09 +1000 From: Anthony Liguori In-Reply-To: <51753471.5080803@redhat.com> References: <1366275680-15416-1-git-send-email-kraxel@redhat.com> <87ppxq6hmn.fsf@blackfin.pond.sub.org> <5174DEE5.20406@redhat.com> <517504CB.6040303@redhat.com> <20130422085043.4a3e891c@redhat.com> <51753471.5080803@redhat.com> Date: Mon, 22 Apr 2013 11:49:58 -0500 Message-ID: <8738uiqzm1.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RfC PATCH 0/5] console: qom-ify & extent screendump monitor command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Luiz Capitulino Cc: qemu-devel@nongnu.org, Gerd Hoffmann , Markus Armbruster Paolo Bonzini writes: > Il 22/04/2013 14:50, Luiz Capitulino ha scritto: >> On Mon, 22 Apr 2013 11:37:15 +0200 >> Paolo Bonzini wrote: >> >>> Il 22/04/2013 08:55, Gerd Hoffmann ha scritto: >>>>>>>> Question for the libvirt guys: Is it ok for libvirt to just extend the >>>>>>>> existing screendump command? Can libvirt figure there is a new >>>>>>>> (optional) parameter? See patch #5. >>>>>> >>>>>> Nope, QMP can't do that. I argued for such capabilities, but the >>>>>> "create a new command" philosophy prevailed. >>>>>> >>>>>> Go forth and multiply commands! And have fun picking command names that >>>>>> aren't fugly. >>>> Oh joy. Lets just enumerate things & use "screendump2" ... >>> >>> QMP can't do that _yet_. >>> >>> Let's fix it instead... >> >> The point is that we have chosen not to do so a while ago. In a nutshell, >> Anthony thinks that we should have the same compatibility contract of >> a C API. > > We've been adding fields to types since 0.15, sometimes in the middle of > a struct (since 1.2). You can safely add fields to the end of a struct. If you think about it in terms of a shipped qmp.h file, it would be: struct CharInfo { }; CharInfo *qmp_query_char(QMPSession *session, Error **errp); But adding parameters to functions breaks the ABI and there's no way around unless you make all parameters passed as a single structure. > If the C API is a requirement, it should also be > a requirement for structs. But there are plenty of ways (some nicer, > some uglier) to have different API versions in C. > > For example, I think the QIDL patch had ways to annotate each parameter > independently. You can annotate each argument with the "first version > this appeared in" and complicate the C API generator to generate > multiple C functions for the same command. > > It is then the downstream's responsibility not to backport extra > arguments without a full-blown rebase to a newer version (the same as > for C libraries). Well this is all well and good in abstract, in practice, we want a new screendump command anyway. It'd be *much* nicer to return the screenshot data via the QMP session instead of writing it to a file. So let's take the opportunity to fix the command. We can also introduce a "format" parameter to allow specifying formats othe than PPM. Regards, Anthony Liguori > > Paolo