Hi Kevin, On Tue, Mar 11, 2014 at 12:06:16PM +0100, Kevin Wolf wrote: > 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.
Ok. > > 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. Agreed. > > 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. Indeed, I took some time yesterday to take a deeper look at QemuOpt users, I saw that converting it all to an entirely new framework would be a massive task and maybe not worth it. Extending QemuOpts to new needs seems to be a more reasonable thing. > > 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). Ok. > > 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); Using json descriptors was something I considered after sending the patches and the feedbacks I already got. That's something that would be useful on cases of generating different outputs from it, like manpages or even fragments of it, it would be more flexible as far as I can see. But yeah, I need to take a look at qapi and see how things work. > > Not sure if we really want to go that far, but perhaps it's some food > for thought. Yeah, sure. Regards... -- Leandro Dorileo
