From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39970) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Spj6l-0007y7-16 for qemu-devel@nongnu.org; Fri, 13 Jul 2012 12:52:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Spj6j-0006gA-Vz for qemu-devel@nongnu.org; Fri, 13 Jul 2012 12:52:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Spj6j-0006g4-NM for qemu-devel@nongnu.org; Fri, 13 Jul 2012 12:52:45 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q6DGqjiA009747 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 13 Jul 2012 12:52:45 -0400 Date: Fri, 13 Jul 2012 13:46:38 -0300 From: Luiz Capitulino Message-ID: <20120713134638.3986ff09@doriath.home> In-Reply-To: <1339575768-2557-1-git-send-email-lersek@redhat.com> References: <1339575768-2557-1-git-send-email-lersek@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 00/17] introduce OptsVisitor, rebase -net/-netdev parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: qemu-devel@nongnu.org On Wed, 13 Jun 2012 10:22:31 +0200 Laszlo Ersek wrote: > Inspired by [1], the first half of this series attempts to implement a new > visitor that should clean up defining and processing command line options. > For a more detailed description, please see "[PATCH 05/17] qapi: introduce > OptsVisitor". >=20 > The second half converts -net/-netdev parsing to the new visitor. The general approach looks fine to me, I've made comments to individual pat= ches and have two general comments: 1. This doesn't build for me: In file included from /home/lcapitulino/work/src/qmp-unstable/net/slirp.c:2= 4:0: /home/lcapitulino/work/src/qmp-unstable/net/slirp.h:41:28: error: unknown t= ype name =E2=80=98QemuOptsList=E2=80=99 /home/lcapitulino/work/src/qmp-unstable/net/slirp.c:741:5: error: no previo= us prototype for =E2=80=98net_slirp_parse_legacy=E2=80=99 [-Werror=3Dmissin= g-prototypes] cc1: all warnings being treated as errors make: *** [net/slirp.o] Error 1 make: *** Waiting for unfinished jobs.... 2. I don't think this should go in through qmp's branch because this is mo= re about QemuOpts than about QMP. I suggest three alternatives: - If you're going to go forward and convert more users, then I think you should open your own branch, send pull requests etc - Go through some -net three - Ask Anthony to apply this directly I'll, of course, review it though