From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54195) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSzsF-0003wC-5a for qemu-devel@nongnu.org; Wed, 26 Mar 2014 22:17:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WSzsA-0006kZ-AC for qemu-devel@nongnu.org; Wed, 26 Mar 2014 22:16:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52779) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSzsA-0006kS-1X for qemu-devel@nongnu.org; Wed, 26 Mar 2014 22:16:50 -0400 Date: Thu, 27 Mar 2014 10:16:44 +0800 From: Amos Kong Message-ID: <20140327021644.GA7531@amosk.info> References: <1395320855-5293-1-git-send-email-akong@redhat.com> <532B563F.7090007@redhat.com> <8761n13pnr.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8761n13pnr.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v3 for 2.0] update names in option tables to match with actual command-line spelling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: libvirt-list@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, pbonzini@redhat.com, afaerber@suse.de On Wed, Mar 26, 2014 at 05:12:08PM +0100, Markus Armbruster wrote: > Eric Blake writes: > > > On 03/20/2014 07:07 AM, Amos Kong wrote: > >> We want to establish a mapping between option name and option table, > >> then we can search related option table by option name. > >> > >> This patch makes all the member name of QemuOptsList to match with > >> actual command-line spelling(option name). > >> > >> [ Important Note ] > >> > >> The QemuOptsList member name values are ABI, changing them can break > >> existing -readconfig configuration files. > >> > >> This patch changes: > >> > >> from to introduced in > >> acpi acpitable 0c764a9 v1.5.0 > >> boot-opts boot 3d3b830 v1.0 > >> smp-opts smp 12b7f57 v1.6.0 > >> > >> All three have calcified into ABI already. > >> > >> I have updated the release note of 2.0 > >> http://wiki.qemu.org/ChangeLog/2.0#ABI_breaking > >> > >> Signed-off-by: Amos Kong > >> --- > > > > The benefit of this patch is that 'query-command-line-options' gains a > > fix where three bogus entries are replaced by their actual command line > > spelling. The drawback is that anyone that doesn't pay attention to the > > ABI break announcement, and expects -readconfig and friends to work > > while using the old spelling, is in for a surprise. But since we have > > prominently documented the change, and since consistency makes life > > nicer, I'm in favor of this patch. > > > > Reviewed-by: Eric Blake > > I'm not thrilled about the ABI break, but avoiding it would probably > take too much code for too little gain. We can add an 'alias_index' field to QemuOptsList, and init it to -1 in qemu_add_opts(). And we only re-set 'alias_index' to 'popt->index' in vl.c(option parsing part) then we can find opts by qemu_options[i].index. We also need to give a warning () if it's group name of added QemuOptsList doesn't match with defined option name. If someone add a new QemuOpts and the group name doesn't match, he/she will get a warning, and call a help function to re-set 'alias_index'. I can send a RFC patch for this. > How can we prevent future violations of the convention "QemuOptsList > member name matches the name of the (primary) option using it for its > argument"? Right now, all we have is /* option name */ tacked to the > member. Which is at best a reminder for those who already know. It's absolutely necessary! > I'd ask for a test catching violations if I could think of an easy way > to code it. Currently I prefer this option, so I will send a V3 with strict checking. -- Amos.