From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57296) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNKW5-000081-Kj for qemu-devel@nongnu.org; Tue, 11 Mar 2014 07:06:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WNKVy-00027d-IS for qemu-devel@nongnu.org; Tue, 11 Mar 2014 07:06:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13777) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNKVy-00027N-9I for qemu-devel@nongnu.org; Tue, 11 Mar 2014 07:06:30 -0400 Date: Tue, 11 Mar 2014 12:06:16 +0100 From: Kevin Wolf Message-ID: <20140311110616.GD3215@dhcp-200-207.str.redhat.com> References: <1394304438-14848-1-git-send-email-l@dorileo.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1394304438-14848-1-git-send-email-l@dorileo.org> Subject: Re: [Qemu-devel] [PATCH RFC 0/2] qemu-arg: general purpose argument parser List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leandro Dorileo Cc: Peter Lieven , Peter Maydell , Fam Zheng , Stefan Weil , Michael Tokarev , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini , Laszlo Ersek Am 08.03.2014 um 19:47 hat Leandro Dorileo geschrieben: > The following patchset introduces a general purpose argument parser and migrates > qemu-img to make use of it. qemu-img is just the first user of it, if we see a > good feedback here I move forward and migrate all the other possible users. I was planning to reply to this in more detail, but it doesn't look like I can find the time to do so, so let me just summarise my thoughts briefly. I do like the idea of simplifying qemu-img's argument parsing, but we shouldn't make the mistake of introducing another option parsing infrastructure and end up with three different coexisting models. If we were to introduce a new framework, we must make sure that all code is converted to it and QemuOpts can be dropped. Now replacing QemuOpts and all of its users is not something that seems to be very productive - it works quite well in those places. So I think we should rather extend QemuOpts to work for qemu-img as well. We would probably need to add a new parser to QemuOpts that parses command line options into a QemuOpts, and extend the definition of them with a couple of new features that are required there (sub-QemuOpts for -o, perhaps enumerations for things like --output=human/json, positional parameters). I see that you also fill the values in (global) variables. The existing code that converts QemuOpts into C variables are the QAPI visitors. So I could imagine that we define all qemu-img options in a JSON schema like qapi-schema.json and generate the struct definitions from it. We would then end up with something like this, where the code to create a QemuImgCreateOptions struct from the QemuOpts would be QAPI generated: void img_create(QemuImgCreateOptions *options, Error **errp); Not sure if we really want to go that far, but perhaps it's some food for thought. Kevin